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

Consistency question, re: handling of exceptions thrown during input validation #28

Closed
rwaldron opened this issue Mar 20, 2020 · 10 comments · May be fixed by #29
Closed

Consistency question, re: handling of exceptions thrown during input validation #28

rwaldron opened this issue Mar 20, 2020 · 10 comments · May be fixed by #29

Comments

@rwaldron
Copy link
Contributor

Currently, any errors that occur as a result of failed input validation are thrown synchronously. This means that safe usage of Atomics.waitAsync() will look like this:

try {
  Atomics.waitAsync(...).then(...);
} catch (error) {
  ...
}

The sync flag in DoWait() could be used to instead handle these errors via IfAbruptRejectPromise(abrupt completion value, promiseCapability).

WDYT?

@rwaldron
Copy link
Contributor Author

@syg @lars-t-hansen I'm in the process of writing tests for this feature and I'm finding that this issue keeps coming up. Can I get some thoughts here?

Also, I want to hear from @ljharb as well.

@ljharb
Copy link
Member

ljharb commented Mar 26, 2020

This is a good catch; async function was explicitly designed to never throw a sync exception, including from default arguments - it seems ideal to follow the same precedent here.

@syg

This comment has been minimized.

@syg
Copy link
Collaborator

syg commented Mar 26, 2020

My current thinking is that validation errors should reject the promise. I'll move the discussion of eagerly returning "not-equal" off of this issue. Found SYNC-RESOLVE.md from 3 years ago where we came down to not having any sync "not-equal" result, though now I have renewed worries about the performance.

@syg
Copy link
Collaborator

syg commented Mar 26, 2020

Though to be clear this is a needs-consensus change. I'll make a PR to the agenda for a deprioritized item.

@syg
Copy link
Collaborator

syg commented Mar 27, 2020

@ljharb I've chatted with some more folks since #28 (comment) and @jridgewell pointed me to https://2ality.com/2016/03/promise-rejections-vs-exceptions.html, which seems well reasoned. That argues that invalidation errors should fail fast and remain synchronous exceptions. Are there API examples where invalidation errors are turned into Promise rejections?

@ljharb
Copy link
Member

ljharb commented Mar 27, 2020

Yes, Promise.all().

@syg
Copy link
Collaborator

syg commented Mar 27, 2020

As another counterpoint to the status quo, I was pointed to https://www.w3.org/2001/tag/doc/promises-guide#always-return-promises.

@rwaldron
Copy link
Contributor Author

rwaldron commented Mar 27, 2020

And Promise.any()

@rwaldron
Copy link
Contributor Author

rwaldron commented Apr 2, 2020

Looks like we have solution

@rwaldron rwaldron closed this as completed Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants