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

Includes invalid values in thrown errors #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

artem-zakharchenko
Copy link

These changes add invalid values into thrown error messages to be more descriptive.

We would like to have this improvement as one of our libraries returns media-typer errors to our users. It would be great to guide users in debugging by including the actual values in the error messages.

References

@artem-zakharchenko
Copy link
Author

Hello, @dougwilson. I hate to disturb you, but could you please give us an estimate if/when these changes can be merged and released? Our team awaits them with trembling. Thank you.

@dougwilson
Copy link
Contributor

Hi @artem-zakharchenko thank you for this pull request. Sorry I did not respond when it was opened. With validation type of errors like this, I like to follow the practice of validation errors should not reflect the user-supplied values in the error message. This can cause various issues, like if the type is invalid because it included a newline, well, now there is a newline in your error message, and err.stack is going to be strange to various folks who are expecting the first line to be the message.

Ideally the caller of this library should have a copy of the invalid value that can be remitted however it likes. But if having the broken up values are valuable, they can be added as properties on the thrown error object so the caller can see what the specifics are.

@dougwilson dougwilson added the pr label Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gavel.js should print an invalid media type
2 participants