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

Consider throwing TypeError for invalid input #583

Closed
a-sully opened this issue Feb 24, 2024 · 0 comments · Fixed by #589
Closed

Consider throwing TypeError for invalid input #583

a-sully opened this issue Feb 24, 2024 · 0 comments · Fixed by #589

Comments

@a-sully
Copy link
Contributor

a-sully commented Feb 24, 2024

Building off of #572 (comment)

DOMException spec for reference: https://webidl.spec.whatwg.org/#idl-exceptions


WebNN currently specifies that a DataError be thrown for all sorts of input validation failures. This is not in line with most other specs, which use TypeError for this.

I'd like to propose that most all platform-agnostic input validation checks (with the MLGraphBuilder operand-vending methods foremost in mind) should throw a TypeError on failure. A non-exhaustive list of validation checks which should throw TypeError include:

  • all steps in the validate MLOperand algorithm
  • anything related to MLOperandDescriptor fields
  • sequence<T> is of a valid length
  • axes/ranks/dimensions/shapes are of the required sizes
  • axes/ranks/dimensions/shapes are compatible with each other
a-sully added a commit to a-sully/webnn that referenced this issue Feb 28, 2024
a-sully added a commit to a-sully/webnn that referenced this issue Feb 28, 2024
a-sully added a commit to a-sully/webnn that referenced this issue Feb 29, 2024
@fdwr fdwr closed this as completed in #589 Mar 1, 2024
fdwr pushed a commit that referenced this issue Mar 1, 2024
* Throw TypeError for most all input validation

A few instances use RangeError

Fixes #583

* make some RangeErrors TypeErrors, and vice versa

* just kidding, remove all RangeErrors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants