-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix: don't break joi validation for unhandledRejections & apirunner #15600
Conversation
// reason can be anything, it can be a message, an object, ANYTHING! | ||
// we convert it to an error object so we don't crash on structured error validation | ||
if (!(reason instanceof Error)) { | ||
reason = new Error(util.format(reason)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no idea how to improve this, so I'm happy to accept suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, seems pretty reasonable 👍
Co-Authored-By: Michal Piechowiak <misiek.piechowiak@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! These look much nicer!
Description
Structured logging as some flaws in our code. These changes don't fix evertyhing but they fix the ones that are mentioned in the issues.
unhandledRejection is a hard event to format into a structured log. The reason why it fails can be anything (ex:
Promise.reject({ errors: ["I don't care"]})
). This PR formats into a string so we at least don't break. UnhandledRejection should only occur on users code and not plugins as we should be guarded against this.The other fix is that I converted our api runner into a structured error as not every error was compatible. We want plugins to use structured logging themselves but for now, this fixes weird errors thrown in plugins.
Related Issues
Fixes #15487
Partially fixes #15242