Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

The request coalescer now throws the AbortSignal's reason #2174

Conversation

steveluscher
Copy link
Contributor

Summary

I learned today that it's wrong to synthesize an AbortError DOMException with an AbortSignal's reason. You should throw the reason directly.

See https://fetch.spec.whatwg.org/#deserialize-a-serialized-abort-reason

@steveluscher
Copy link
Contributor Author

steveluscher commented Feb 24, 2024

@steveluscher steveluscher force-pushed the 02-24-Eliminate_jest-fetch-mock-fork_ branch from 8b39005 to ce9d0f3 Compare February 24, 2024 07:10
@steveluscher steveluscher force-pushed the 02-24-The_request_coalescer_now_throws_the_AbortSignal_s_reason_ branch from a1db3c6 to 148902b Compare February 24, 2024 07:10
@steveluscher steveluscher force-pushed the 02-24-Eliminate_jest-fetch-mock-fork_ branch from ce9d0f3 to 87662a3 Compare February 24, 2024 07:28
@steveluscher steveluscher force-pushed the 02-24-The_request_coalescer_now_throws_the_AbortSignal_s_reason_ branch from 148902b to b19be46 Compare February 24, 2024 07:28
Base automatically changed from 02-24-Eliminate_jest-fetch-mock-fork_ to master February 24, 2024 07:42
@steveluscher steveluscher force-pushed the 02-24-The_request_coalescer_now_throws_the_AbortSignal_s_reason_ branch from b19be46 to 30e573a Compare February 26, 2024 05:49
Copy link
Contributor

@lorisleiva lorisleiva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I also didn't know this was the recommended way to manually abort.

@@ -8,6 +8,13 @@ type CoalescedRequest = {

type GetDeduplicationKeyFn = (payload: unknown) => string | undefined;

const EXPLICIT_ABORT_TOKEN = Symbol(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this a SolanaError instead? I'm a bit scared of global Symbols because of dual package hazards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fully internal to this module. I throw it through the internal AbortController that's attached to the internal fetch.

@steveluscher steveluscher merged commit 356279e into master Feb 27, 2024
8 of 10 checks passed
@steveluscher steveluscher deleted the 02-24-The_request_coalescer_now_throws_the_AbortSignal_s_reason_ branch February 27, 2024 18:27
Copy link
Contributor

github-actions bot commented Mar 2, 2024

🎉 This PR is included in version 1.90.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants