Skip to content

script: add error messages#41039

Merged
jdm merged 3 commits intoservo:mainfrom
danilopedraza:issue-40756
Dec 5, 2025
Merged

script: add error messages#41039
jdm merged 3 commits intoservo:mainfrom
danilopedraza:issue-40756

Conversation

@danilopedraza
Copy link
Copy Markdown
Contributor

script: add error messages for conditions in encrypt_aes_ctr

Testing: No tests needed (?) let me know if I should add tests for this somewhere.
Fixes: #40756

Followed the MDN and w3c docs to write the messages.

I'm thinking on adding more messages, so any feedback would be appreciated! :)

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 4, 2025
@jdm
Copy link
Copy Markdown
Member

jdm commented Dec 4, 2025

@kkoyung Want to review this?

@kkoyung
Copy link
Copy Markdown
Member

kkoyung commented Dec 4, 2025

@kkoyung Want to review this?

Sure!

Copy link
Copy Markdown
Member

@kkoyung kkoyung 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 small change.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 4, 2025
Signed-off-by: César Pedraza <[email protected]>
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Dec 4, 2025
Copy link
Copy Markdown
Member

@kkoyung kkoyung left a comment

Choose a reason for hiding this comment

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

Thanks!

I'm thinking on adding more messages, so any feedback would be appreciated! :)

In the WebCrypto code (most of which is in components/script/dom/subtlecrypto/), there are a lot of steps that are "If XXX, then throw an YYYError", making it easy to write the error message for them. You can check it out.
Other parts in our codebase are missing many error messages too. Contribution to them are also welcome.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 4, 2025
@jdm jdm added this pull request to the merge queue Dec 5, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 5, 2025
Merged via the queue into servo:main with commit 6e83848 Dec 5, 2025
29 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide actual messages in JS errors

4 participants