fix disturbed ReadableStream error #65
Conversation
| @@ -91,17 +91,9 @@ export class AwsClient { | |||
| input = url | |||
| } | |||
| const signer = new AwsV4Signer(Object.assign({ url: input }, init, this, init && init.aws)) | |||
| const signed = Object.assign({}, init, await signer.sign()) | |||
| const signed = Object.assign(signer.body instanceof ReadableStream ? { duplex: 'half' } : {}, init, await signer.sign()) | |||
There was a problem hiding this comment.
potentially we could always include { duplex: 'half' }, I'm not sure how necessary it is to actually run the signer.body instanceof ReadableStream check.
| return new Request(signed.url.toString(), signed) | ||
| } catch (e) { | ||
| if (e instanceof TypeError) { | ||
| // https://bugs.chromium.org/p/chromium/issues/detail?id=1360943 |
There was a problem hiding this comment.
As you can see from here – it's a Chrome workaround specifically.
You'd need to check that { duplex: 'half' } works on all platforms (and that it's appropriate)
|
@jamesthomp there might be something else going on here – namely that there is a TypeError being thrown for a different reason during the construction of the Request (or that it's being thrown because the stream has already been read from, and specifying |
|
Thanks for taking a look - I'll close this out if you don't believe its the right fix. Hopefully I'll catch the original TypeError the next time this happens to understand this problem better |
Fixes #52
When using this in the Cloudflare Workers runtime, I noticed an error:
The issue doesn't reliably reproduce, but given the error message I suspect that the first Request on line 55 in the screenshot is consuming some of the body ReadableStream, which causes it to then throw on line 58.
The fetch spec states that the duplex property must be set if the body is a ReadableStream:
So I've adjusted the code to do this