Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue: synchronous exceptions in JS API #225

Closed
7 of 8 tasks
kainino0x opened this issue Sep 7, 2023 · 5 comments · Fixed by #377
Closed
7 of 8 tasks

Tracking issue: synchronous exceptions in JS API #225

kainino0x opened this issue Sep 7, 2023 · 5 comments · Fixed by #377
Labels
errors Error reporting has resolution Issue is resolved, just needs to be done wasm Issues with WebAssembly targets

Comments

@kainino0x
Copy link
Collaborator

kainino0x commented Sep 7, 2023

I'll attempt to make a complete list of entrypoints that have synchronous exceptions in the JS API:

  • All cases where you try to use a feature that's not enabled on the device that would throw a TypeError on a browser that hasn't implemented the feature at all (see spec)
    • Using an enum value introduced by the feature (e.g. query type "timestamp")
    • Calling a method introduced by the feature (e.g. writeTimestamp)
  • getMappedRange: not mapped; not aligned; out of range; overlaps (Synchronous validation in getMappedRange #64)
  • createBuffer with mappedAtCreation (OOM-ish conditions) (Signaling allocation failure for buffers with mapAtCreation==true #57)
  • external image stuff (copyExternalImageToTexture/importExternalTexture) (not in C API yet, ExternalTexture in WASM #232):
    • "origin-clean" and "check the usability" of external image sources
    • copyExternalImageToTexture source image size check (can be checked in the bindings beforehand so it’s possible to know what the error is, except maybe for HTMLVideoElement because it’s weird and frames advance in the background)
  • writeBuffer: size alignment. Maybe this should be a validation error in the upstream spec?
  • getCurrentTexture: not configured. See proposal: swapchain & presentation rework #197, maybe we need an extra GetCurrentTextureStatus for this?
  • Things that can't happen from C
    • Invalid extent/origin/color arrays
    • Errors about the size of a source BufferSource in writeBuffer/writeTexture/setBindGroup
  • (EDIT 2024-10-18) Invalid formats for configure() - this was recently moved from device timeline to content timeline in the JS spec.

One approach we've been taking for some entrypoints is to say "return a generic error code, with implementation-defined logging" (eventually we're likely to make it well-defined, with a log callback on the instance):

@kainino0x kainino0x added the errors Error reporting label Sep 7, 2023
@kainino0x
Copy link
Collaborator Author

Note this is in addition to synchronous errors from the C API that don't exist in the JS API (like #115).

@kainino0x
Copy link
Collaborator Author

kainino0x commented Dec 15, 2023

Dec 14 meeting:

  • Using features that aren’t enabled
    • KN: propose injecting an error
    • OK!
  • getCurrentTexture when not configured
    • KN: Add a new status and return it?
    • (discussion) Return with a null texture?
    • Add a new status and return null texture (not error texture, WASM wouldn't be able to do that)
  • writeBuffer size alignment
    • Inject an error, then we don’t care what the JS API does

@kainino0x kainino0x added the has resolution Issue is resolved, just needs to be done label Dec 15, 2023
@kainino0x
Copy link
Collaborator Author

Marking "has resolution", we can figure out #232 when we get to it

kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
@kainino0x
Copy link
Collaborator Author

kainino0x commented Oct 19, 2024

One new addition:

  • Invalid formats for configure() - this was recently moved from device timeline to content timeline in the JS spec.

@kainino0x
Copy link
Collaborator Author

kainino0x commented Oct 19, 2024

I handled that in #377 by making it also a device error in webgpu.h (same as for formats not supported by the surface, and formats not supported by the device)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Error reporting has resolution Issue is resolved, just needs to be done wasm Issues with WebAssembly targets
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant