fix: don't cancel with invalid state #1138
Conversation
lib/fetch/index.js
Outdated
| if (request.body != null) { | ||
| cancelBody(request.body, error) | ||
| if (request.body != null && isReadable(request.body?.stream)) { | ||
| request.body.stream.cancel(error) |
There was a problem hiding this comment.
@szmarczak: We have another spec problem here... the stream might be locked at which point we are not allowed the call cancel... even though the stream is readable... ideas?
There was a problem hiding this comment.
@jasnell I think actually there might be a bug in node's implementation. It doesn't say anything about throwing invalid state if the stream is locked when cancelling? https://streams.spec.whatwg.org/#readable-stream-cancel
There was a problem hiding this comment.
✖ Invalid state: ReadableStream is locked
test: TAP
at:
line: 371
column: 5
file: node:internal/errors
constructor: true
function: NodeError
stack: >
EventEmitter.abortFetch (lib/fetch/index.js:292:26)
EventTarget.requestObject.signal.addEventListener.once (lib/fetch/index.js:144:18)
EventTarget.Request.signal.addEventListener.once (lib/fetch/request.js:335:16)
type: TypeError
code: ERR_INVALID_STATE
tapCaught: unhandledRejectionThere was a problem hiding this comment.
@szmarczak also... the spec doesn't say what to do if the promise returned from cancel rejects...
There was a problem hiding this comment.
the stream might be locked at which point we are not allowed the call cancel
Locking just means it's "piped" to a reader. Cancellation still should be possible.
const controller = new AbortController();
const response = await fetch('https://speed.hetzner.de/100MB.bin', {signal: controller.signal});
const reader = response.body.getReader();
console.log(response.body.locked);
controller.abort();
console.log(await reader.read()); // GoogleChrome - DOMException: The user aborted a request.There was a problem hiding this comment.
the spec doesn't say what to do if the promise returned from cancel rejects
I think it never rejects.
Codecov Report
@@ Coverage Diff @@
## main #1138 +/- ##
=======================================
Coverage 94.25% 94.26%
=======================================
Files 40 40
Lines 3797 3798 +1
=======================================
+ Hits 3579 3580 +1
Misses 218 218
Continue to review full report at Codecov.
|
ronag
force-pushed
the
1138
branch
2 times, most recently
from
28f3913 to
60ca745
Compare
* fix: don't cancel with invalid state * fixup * fixup
* fix: don't cancel with invalid state * fixup * fixup
* fix: don't cancel with invalid state * fixup * fixup
Refs: #1137