Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't throw in _transform of BulkLoad #1590

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

everhardt
Copy link

I got an uncaughtException, caused by trying to write a very large (negative) number using BulkLoad to a decimal column. Seems similar to #421. The stack trace I got:

RangeError: The value of "value" is out of range. It must be >= 0 and <= 4294967295. Received 3_.40_282_346_638_528_86e_+42
  File "node:internal/errors", line 496, in __node_internal_captureLargerStackTrace
  File "node:internal/errors", line 405, in new NodeError
  File "node:internal/buffer", line 74, in checkInt
  File "node:internal/buffer", line 694, in writeU_Int32LE
  File "node:internal/buffer", line 707, in Buffer.writeUInt32LE
  File "/app/node_modules/tedious/src/data-types/decimal.ts", line 77, in Object.generateParameterData
  File "<anonymous>", line unknown, in generateParameterData.next
  File "/app/node_modules/tedious/src/bulk-load.ts", line 209, in _transform
  File "node:internal/streams/transform", line 175, in Transform._write
  File "node:internal/streams/writable", line 392, in writeOrBuffer
  File "node:internal/streams/writable", line 333, in _write
  File "node:internal/streams/writable", line 337, in Writable.write
  File "node:internal/streams/readable", line 777, in Readable.ondata
  File "node:events", line 517, in Readable.emit
  File "node:internal/streams/readable", line 550, in Readable.read
  File "node:internal/streams/readable", line 1032, in flow
  File "node:internal/streams/readable", line 1013, in resume_
  File "node:internal/process/task_queues", line 82, in process.processTicksAndRejections

bulk-load:209 is inside the _tranform function, for which I think the following in the nodejs documentation holds:

Throwing an Error from within these methods or manually emitting an 'error' event results in undefined behavior.

Therefore, I propose to catch such errors and call the callback with it. I verified that solved my case.

@arthurschreiber
Copy link
Collaborator

Can you add tests for this change?

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (57998f2) 79.00% compared to head (b3b035c) 78.08%.

❗ Current head b3b035c differs from pull request most recent head d704f1b. Consider uploading reports for the commit d704f1b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1590      +/-   ##
==========================================
- Coverage   79.00%   78.08%   -0.93%     
==========================================
  Files          93       93              
  Lines        4821     4823       +2     
  Branches      921      921              
==========================================
- Hits         3809     3766      -43     
- Misses        709      756      +47     
+ Partials      303      301       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@everhardt
Copy link
Author

Can you add tests for this change?

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants