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.

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

[spec] Error type for fetch cancellation #757

Closed
gahaas opened this issue Mar 21, 2018 · 11 comments
Closed

[spec] Error type for fetch cancellation #757

gahaas opened this issue Mar 21, 2018 · 11 comments

Comments

@gahaas
Copy link
Collaborator

gahaas commented Mar 21, 2018

As far as I understand web-platform-tests/wpt#10096, the fetch API throws an AbortError instead of a TypeError when fetch gets cancelled. At the moment the WebAssembly spec says that we throw a TypeError. Is this something we want to change in the spec so that we handle this case the same as the fetch API?

@annevk
Copy link
Member

annevk commented Mar 23, 2018

This would only matter for http://webassembly.github.io/spec/web-api/index.html#compile-a-potential-webassembly-response right? And it seems to me that already forwards the underlying exception correctly.

@gahaas
Copy link
Collaborator Author

gahaas commented Mar 26, 2018

Hmm, I was referring to: "... If the Response ... does not represent an ok status, ... , the returned promise will be rejected with a TypeError". Do you mean the underlying exception is forwarded because of "8. Upon rejection of bodyPromise with reason reason: Reject returnValue with reason."?

I'm not convinced that the underlying is forwarded because of the following reasons:

The fetch API says in https://fetch.spec.whatwg.org/#fetching
2. If the ongoing fetch is terminated, then:
2.1. Let aborted be the termination’s aborted flag.
2.2. If aborted is set, then return an aborted network error.

And in https://fetch.spec.whatwg.org/#responses the API says:
A network error is a response whose status is always 0.

Therefore it seems to me that if fetching is terminated by an abort, then the Response does not represent an ok status, and we have to throw a TypeError and not an AbortError.

@annevk
Copy link
Member

annevk commented Mar 26, 2018

Those responses are an internal concept. Response objects never represent a network error. If you get a Response object you can be certain that it represents something you either created yourself or you got from the network.

@gahaas
Copy link
Collaborator Author

gahaas commented Mar 26, 2018

Are you saying that the Response still represents an ok status, even when fetch got cancelled?

@annevk
Copy link
Member

annevk commented Mar 26, 2018

@gahaas if you got a Response object already, then yes, only the Response's body (a stream) will then be canceled.

@littledan
Copy link
Collaborator

Does this leave open other possible reasons for having a non-ok status? I'd be fine with switching to any other error for this case; I was just using the error type that was there in the previous specification.

@annevk
Copy link
Member

annevk commented Apr 4, 2018

Using TypeError for non-ok seems totally fine. If it's aborted you either don't have a Response or its stream will be aborted (with the correct exception). I don't think there's an issue here.

@littledan
Copy link
Collaborator

OK, seems like we are in agreement here that an AbortError should be thrown here. Should we add a note in the spec to make it more obvious? @Ms2ger could you ensure that we have a test for this case in wpt?

@Ms2ger
Copy link
Collaborator

Ms2ger commented Oct 22, 2018

@ericprud
Copy link

@gahaas, @Ms2ger, if web-platform-tests/wpt#13653 addresses this, please close this issue. tx.

@littledan
Copy link
Collaborator

Closing per #757 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants