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

more user-friendly jsg exception message #2532

Merged
merged 1 commit into from
Aug 19, 2024
Merged

more user-friendly jsg exception message #2532

merged 1 commit into from
Aug 19, 2024

Conversation

mikea
Copy link
Collaborator

@mikea mikea commented Aug 14, 2024

Strictly speaking this is not backward compatible, but I doubt anyone cares and it has saved me some time while debugging an issue.

Strictly speaking this is not backward compatible, but I doubt anyone cares and it has saved me some time while debugging an issue.
@mikea mikea requested review from jasnell and anonrig August 14, 2024 14:31
@mikea mikea requested review from a team as code owners August 14, 2024 14:31
@@ -103,8 +103,9 @@ class StructWrapper<Self, T, TypeTuple<FieldWrappers...>, kj::_::Indexes<indices
kj::isSameType<typename FieldWrappers::Type, Unimplemented>()) && ...)) {
return T{};
}
jsg::throwTypeError(isolate, "Cannot initialize a dictionary with required members from an "
"undefined or null value.");
jsg::throwTypeError(isolate, kj::str("Cannot initialize ", typeid(T).name(),
Copy link
Member

Choose a reason for hiding this comment

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

minor nit but is this going to give a user friendly name that is useful for end users or something only we can appreciate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Friendliness is debatable, but this is definitely something that will have a name similar to TS type.

@mikea mikea merged commit 8b8f52d into main Aug 19, 2024
8 of 9 checks passed
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.

4 participants