Skip to content

Conversation

@yaacovCR
Copy link
Contributor

should encompass thrown errors and encountered abort errors

@yaacovCR yaacovCR requested a review from a team as a code owner October 27, 2024 18:57
@netlify
Copy link

netlify bot commented Oct 27, 2024

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit 3c2c21d
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/671e8d7d1c95150008f96eba
😎 Deploy Preview https://deploy-preview-4260--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions
Copy link

Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@yaacovCR yaacovCR force-pushed the make-to-error-more-generic branch from f6ed24a to 3c2c21d Compare October 27, 2024 18:59
@yaacovCR yaacovCR marked this pull request as draft October 27, 2024 18:59
@yaacovCR
Copy link
Contributor Author

This PR emerged from #4258 => but note that comment at #4258 (comment) by @JoviDeCroock notes remaining reservation with respect to the modified prefix.

I have left this PR in draft, because considering the above mentioned reservation, I am not sure if we consider it better than the status quo.

An argument could be made that the status quo is sufficient: although the AbortError is not technically thrown, depending on the implementation, it could have been, as we could have implemented it as a thrown error simply by using abortSignal.throwIfAborted() => we chose instead to directly access abortSignal.aborted and abortSignal.reason directly, for performance reasons on some runtimes, but semantically, the abortSignal is a "throwable" value.

This PR is also another breaking change, technically, although hopefully it doesn't hit too many people, as since I do not like our current state of error wrapping at all, see discussion at

@JoviDeCroock JoviDeCroock added the PR: polish 💅 PR doesn't change public API or any observed behaviour label Feb 6, 2025
@yaacovCR
Copy link
Contributor Author

Closing this for now, we can always reopen if desired.

@yaacovCR yaacovCR closed this Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: polish 💅 PR doesn't change public API or any observed behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants