fetch: implement new spec changes #1721
Conversation
Codecov ReportBase: 89.55% // Head: 89.60% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1721 +/- ##
==========================================
+ Coverage 89.55% 89.60% +0.04%
==========================================
Files 58 58
Lines 5257 5263 +6
==========================================
+ Hits 4708 4716 +8
+ Misses 549 547 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
lib/fetch/body.js
Outdated
| controller.enqueue( | ||
| typeof source === 'string' ? new TextEncoder().encode(source) : source | ||
| ) | ||
| queueMicrotask(() => { |
There was a problem hiding this comment.
Yes, this is a known issue with node readable streams. I think we have the same queueMicrotask hack somewhere else.
There was a problem hiding this comment.
Maybe we could move this into a closeReadableStream helper function and re-use? Then we can link additional information why that is needed in one place.
There was a problem hiding this comment.
Yeah I see it somewhere else, I actually copied it from there lol
Do you know why it happens? Some places don't use try/catch and some places do
There was a problem hiding this comment.
I don't remember. Maybe @jasnell has an idea. I believe there is some issue over at the node repo.
Implements