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
AbortController abort reason Parameter #1462
Comments
Im not sure if I understood the ecosystem correctly, but i want to point out that i also opened the same issue as discussion for @types/node. I hope thats alright. As said in the comments on the discussion: I hope i raised this correctly, if not, please help me do it right 😄 |
Must say that this issue was a bit fussy, didn't exactly understood what you wanted us to do... The signal.reason was added in node v17.2.0 |
Sorry for unclearity. I wasnt aware that its so new, because in the abort documentation they just say, that the parameter exists and I did not see that there is a specific page for AbortSignal.reason. Yes it seems to be quite new, but it also seems quite handy to me. Thats why I opened the Issue to give the pointer. ^^ |
You can try and use the following snippet try{
fetch...
} catch (error) {
if (abortController.signal.aborted) {
return 'http request was timed out';
}
} |
this approach is incredibly imprecise if the fetch could fail for multiple reasons, or if the AbortSignal could be triggered for different reasons (e.g. timeout vs process shutdown). Unless we can actually use the So I agree with OP that it would be very valuable to have node-fetch respect the |
I believe the specifications says that, upon abort, the reason (when given) would be thrown instead of the default For me this is very invasive. It prevents detecting that an abort has occurred regardless of reason. A middle ground would be to set an error's try {
const controller = new AbortController();
const timeout = setTimeout(() => { controller.abort("took to long"); }, 1000 * 60);
const response = await fetch("http://takes.very-long-time-to-resepond.com/api", {
signal: contoller.signal,
}).finally(() => clearTimeout(timeout));
/* use response */
} catch (error) {
if (error instanceof AbortError) {
console.log("fetch aborted");
if (error.cause === "took to long") {
console.log("because it took so long");
} else {
throw new Error("Unknown abortion", { cause: error });
}
} else {
throw new Error("Unknown error", { cause: error });
}
} Alternatively, the But never throw the reason. |
Node's implementation of |
Description
In the definition of the AbortController on MDN it is stated, that it is possible to pass a parameter to
AbortController.abort()
that is thrown as Error if the AbortController is used in a fetch call.Current
AbortController.abort()
exists, but does not expect a parameter.Wanted
Like done in the [@types/mongodb] implementation (see here), the optional reason parameter is also needed in the [@types/node] implementation. And i think before that makes sense, it has to be implemented here in node-fetch
Version
Im using node-fetch as a transitive dependency on version
2.6.6
.I Set up a project using next.js using
^12.0.7
and ts-node using^10.4.0
The text was updated successfully, but these errors were encountered: