Skip to content

Conversation

@kainino0x
Copy link
Contributor

@kainino0x kainino0x commented Oct 12, 2022

Proposed change

Fixes #3296 (note #3297 remains open post-v1)

@kainino0x kainino0x added the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Oct 12, 2022
@kainino0x kainino0x added this to the V1.0 milestone Oct 12, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 12, 2022

Previews, as seen when this build job started (d26e47c):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

@kainino0x kainino0x added for webgpu editors meeting tacit resolution candidate Editors may be able to resolve and move to tacit resolution queue labels Oct 13, 2022
@kainino0x
Copy link
Contributor Author

cc @toji @litherum

I started updating this to have an exception object that we could extend in the future to provide structured error data if we want to. However, I ended up realizing there are guidelines that prevent us from doing so.

WebIDL doesn't allow defining custom exception types:

An exception is a type of object that represents an error and which can be thrown or treated as a first class value by implementations. Web IDL does not allow exceptions to be defined, but instead has a number of pre-defined exceptions that specifications can reference and throw in their definition of operations, attributes, and so on. Exceptions have an error name, a DOMString, which is the type of error the exception represents, and a message, which is an optional, user agent-defined value that provides human readable details of the error.
https://webidl.spec.whatwg.org/#idl-exceptions

Meanwhile the Promises guide says:

Rejection reasons must be Error instances
Promise rejection reasons should always be instances of the JavaScript Error type, just like synchronously-thrown exceptions should always be instances of Error. This generally means using either one of the built-in JavaScript error types, or using DOMException.
https://www.w3.org/2001/tag/doc/promises-guide#reasons-should-be-errors

I believe this doc is more-or-less authoritative. Unfortunately, the somewhat more maintained Design Principles don't say much:
https://w3ctag.github.io/design-principles/#error-types

Technically, I'm sure it's possible to create a plain Error object, hang properties off of it, and return it, but that's a workaround. The Promises guide is a pretty good indicator we're not expected to go in this direction.

I haven't thought of any good alternatives yet.

@kainino0x kainino0x added for webgpu editors meeting tacit resolution queue Editors have agreed and intend to land if no feedback is given and removed copyediting Pure editorial stuff (copyediting, *.bs file syntax, etc.) for webgpu editors meeting tacit resolution candidate Editors may be able to resolve and move to tacit resolution queue labels Nov 23, 2022
@kainino0x kainino0x requested a review from toji November 28, 2022 22:23
@kainino0x kainino0x marked this pull request as ready for review November 28, 2022 22:23
Copy link
Member

@toji toji left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of nits

@kainino0x kainino0x added copyediting Pure editorial stuff (copyediting, *.bs file syntax, etc.) and removed tacit resolution queue Editors have agreed and intend to land if no feedback is given labels Dec 7, 2022
@kainino0x kainino0x enabled auto-merge (squash) December 30, 2022 02:15
@kainino0x kainino0x merged commit 0e99c8f into gpuweb:main Dec 30, 2022
@kainino0x kainino0x deleted the pipeline-reject branch December 30, 2022 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copyediting Pure editorial stuff (copyediting, *.bs file syntax, etc.) needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

createPipelineAsync() doesn't reject its promise, even if the created pipeline is invalid

3 participants