[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

[BUG] Error thrown on empty body responses (e.g. 304) #15

Closed
asos-craigmorten opened this issue Aug 24, 2020 · 0 comments · Fixed by #16
Closed

[BUG] Error thrown on empty body responses (e.g. 304) #15

asos-craigmorten opened this issue Aug 24, 2020 · 0 comments · Fixed by #16
Assignees
@asos-craigmorten
Labels
bug Something isn't working

Comments

@asos-craigmorten
Copy link
Collaborator
asos-craigmorten commented Aug 24, 2020 •
edited
Loading

Issue

Setup:

  • Deno Version: 1.3.1
  • v8 Version: 8.6.334
  • Typescript Version: 3.9.7
  • SuperDeno Version: 2.2.0

Seeing 304 Not Modified responses with empty (no) body result in an error thrown by SuperDeno.

error: Uncaught Error: Request has been terminated
Possible causes: the network is offline, Origin is not allowed by Access-Control-Allow-Origin, the page is being unloaded, etc.
    at Test.Request.crossDomainError (https://dev.jspm.io/npm:superagent@5.3.1/lib/client.dew.js:680:15)
    at XMLHttpRequestSham.xhr.onreadystatechange (https://dev.jspm.io/npm:superagent@5.3.1/lib/client.dew.js:783:21)
    at XMLHttpRequestSham.xhrReceive (https://deno.land/x/superdeno@2.2.0/src/xhrSham.js:105:29)
    at XMLHttpRequestSham.send (https://deno.land/x/superdeno@2.2.0/src/xhrSham.js:64:12)

Details

Adding some logs in the xhrSham we can see that (xhrSham.js:293:34):

...

window._xhrSham.promises[self.id] = fetch(options.url, {
  method: options.method,
  headers: options.requestHeaders,
  body,
  signal: this.controller.signal,
  mode: "cors",
});

// Wait on the response, and then read the buffer.
response = await window._xhrSham.promises[self.id];

...

const buf = await response.arrayBuffer();
parsedResponse = decoder.decode(buf); // BOOM!

...

Is throwing:

err: TypeError: Cannot use 'in' operator to search for 'buffer' in null
    at TextDecoder.decode (/Users/runner/work/deno/deno/op_crates/web/08_text_encoding.js:445:18)
    at XMLHttpRequestSham.xhrSend (https://deno.land/x/superdeno@2.2.0/src/xhrSham.js:293:34)
    at async XMLHttpRequestSham.send (https://deno.land/x/superdeno@2.2.0/src/xhrSham.js:51:7)

This doesn't occur in Deno 1.3.0, so assume introduced by one of the changes to Deno core in 1.3.1, see the release notes: https://github.com/denoland/deno/releases/tag/v1.3.1

08_text_encoding line ref: https://github.com/denoland/deno/blob/master/op_crates/web/08_text_encoding.js#L445

Reproducing we can see we get the error with:

const decoder = new TextDecoder("utf-8");
const buf = null;
const decoded = decoder.decode(buf as any);

It would seem that response.arrayBuffer() in Deno 1.3.0 returned "" for a null body whereas no it returns null which causes the error in the decoder as it has no null protection.

We can protect against this in SuperDeno by adding our own null check around the decoder.
@asos-craigmorten asos-craigmorten self-assigned this Aug 24, 2020
@asos-craigmorten asos-craigmorten added the bug Something isn't working label Aug 24, 2020
@asos-craigmorten asos-craigmorten mentioned this issue Aug 24, 2020
Merged
2 tasks
@asos-craigmorten asos-craigmorten linked a pull request Aug 24, 2020 that will close this issue
[#15] Bump versions and fix null response body bug #16
Merged
2 tasks
asos-craigmorten added a commit that referenced this issue Aug 24, 2020
@asos-craigmorten
[#15] Bump versions and fix null response body bug(#16)
d406d29 
- feat: update to Deno 1.3.1, std 0.66.0 and other dep upgrades.
- fix: [#15] Error thrown on empty body responses (e.g. 304)
@asos-craigmorten asos-craigmorten mentioned this issue Aug 24, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@asos-craigmorten asos-craigmorten

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#15] Bump versions and fix null response body bug cmorten/superdeno
1 participant
@asos-craigmorten