Skip to content

AbortSignal.timeout inner exception should be DISCONNECTED - #1177

Merged
jasnell merged 1 commit into
mainfrom
jsnell/abortsignal-timeout-disconnected
Oct 3, 2023
Merged

AbortSignal.timeout inner exception should be DISCONNECTED#1177
jasnell merged 1 commit into
mainfrom
jsnell/abortsignal-timeout-disconnected

Conversation

@jasnell

@jasnell jasnell commented Sep 13, 2023

Copy link
Copy Markdown
Collaborator

Fixes: #1020

Previously, AbortSignal.timeout() would cause the AbortSignal to trigger with an exception not marked as DISCONNECTED. This would mean that when the signal is used to cancel, for instance, a fetch POST, the exception passed on via the internal canceler would end up being logged as an uncaught exception as reported in the github issue. This commit updates things such that AbortSignal.timeout properly reports a DISCONNECTED kj::Exception when the timeout fires. It also makes that path slightly more efficient by avoiding an extraneous kj::Exception->JS exception->kj::Exception round trip.

Comment thread src/workerd/api/basics.c++
Fixes: #1020

Previously, `AbortSignal.timeout()` would cause the `AbortSignal`
to trigger with an exception not marked as `DISCONNECTED`. This
would mean that when the signal is used to cancel, for instance, a
fetch POST, the exception passed on via the internal canceler would
end up being logged as an uncaught exception as reported in the
github issue. This commit updates things such that AbortSignal.timeout
properly reports a `DISCONNETED` kj::Exception when the timeout fires.
@jasnell
jasnell force-pushed the jsnell/abortsignal-timeout-disconnected branch from f6d1b37 to 7a28ec6 Compare October 3, 2023 16:51
@jasnell
jasnell merged commit 10a45dd into main Oct 3, 2023
@jasnell
jasnell deleted the jsnell/abortsignal-timeout-disconnected branch October 3, 2023 18:02
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

Successfully merging this pull request may close these issues.

🐛 Bug Report — Runtime APIs — AbortSignal.timeout throws un-catchable async DOMException

4 participants