fix: make sure the handler resolves in all cases #507
Conversation
This reverts commit 17ec56c.
There was a problem hiding this comment.
Seems like it was a wrong choice to use while for generator.
I knew it will cause problem somehow, that's why I prefer to use AsyncIterator
Something similar can be done by,
https://github.com/kaka-ng/fastify-plugins/blob/main/packages/multipart/lib/adapter/busboy.ts#L116-L156
|
I don't get an error on node 16, 18, 20, or 21 What is up with 14?.. |
This reverts commit 35aca1e.
|
Seems there are issue on Node v14 |
|
Should I merge to next? |
|
Can you shortly describe why it fails on 14? |
|
I can't for the moment 🤣 Essentially, the test I added doesn't pass on node 14 (time outs I believe), and it needs investigation |
|
Note that it's not because of what this PR does, it's because the PR doesn't seem to fix the issue on node 14 |
|
Seems like on node 14 request.raw.on('error') is not firing... anyone has a guess on why that might be? Was it added recently? |
|
I think the destroy logic in v14 is really different. Maybe let's target |
|
I can't because it's on |
|
a patch? This is in the next branch? Is that ready to ship? |
Yeah, forgot for a second It's not ready IMO, we can wait till v5 since maybe I'll be able to do the major rewrite... unless someone needs this patch ASAP |
|
Thanks @gurgunday (and reviewers) for working on this fix. I understand that there are some issues with getting this released due to node 14, but is there any chance to get this released in a bug-fix release? Or do you have a rough timeline for the v5 release? |
|
@mcollina, should I add a skip to the test and backport it to main? Or would you prefer releasing next? |
* fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci
* fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci
* update for Fastify v5 (#505) * fix: make sure the handler resolves in all cases (#507) * fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci * update for Fastify v5 (#505) * fix: make sure the handler resolves in all cases (#507) * fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci * update fastify deps * update for Fastify v5 (#505) * fix: make sure the handler resolves in all cases (#507) * fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci * Update index.js Co-authored-by: Gürgün Dayıoğlu <[email protected]> Signed-off-by: James Sumners <[email protected]> --------- Signed-off-by: James Sumners <[email protected]> Co-authored-by: Gürgün Dayıoğlu <[email protected]> Co-authored-by: Gürgün Dayıoğlu <[email protected]>
partially fixes #497
The request still freezes without the
try catchand I'm not sure how we could remedy that