Add future flag to throw request.signal.reason for aborted requests #11104
Conversation
🦋 Changeset detectedLatest commit: 219def2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| ) { | ||
| let method = isRouteRequest ? "queryRoute" : "query"; | ||
| if (future.v7_throwAbortReason && request.signal.reason !== undefined) { | ||
| throw request.signal.reason; |
There was a problem hiding this comment.
The only downside I see here is that we lose the request method/URL that we include in the custom error, but I think if folks need that then they should be passing a custom reason when they call controller.abort. Right now, in Remix we don't expose that for customization - but we could send our own DOMException with the method/URL in the message if we wanted to.
| // Note this works in Node 18+ - but it does not work if using the | ||
| // `abort-controller` polyfill which doesn't yet support a custom `reason` | ||
| // See: https://github.com/mysticatea/abort-controller/issues/33 | ||
| controller.abort(new Error("Oh no!")); |
There was a problem hiding this comment.
Worth calling out - this could be an issue in Remix apps that still use the polyfills if we wanted to use a custom reason
|
🤖 Hello there, We just published version Thanks! |
|
🤖 Hello there, We just published version Thanks! |
Closes #9629
See also: