-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Support empty string messages #134
Conversation
index.js
Outdated
@@ -27,7 +27,7 @@ function createError (code, message, statusCode = 500, Base = Error) { | |||
this.cause = args.pop().cause | |||
} | |||
|
|||
this.message = format(message, ...args) | |||
this.message = format(message, ...args).trim() |
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.
Not sure which one will be better, most of the time the if
clause will be executed.
this.message = format(message, ...args).trim() | |
if (message) args.unshift(message) | |
this.message = format(...args) |
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.
I would:
if (typeof message !== 'string') throw Error('Fastify error message must be defined')
(line 11)if (message === '') { this.message = message } else { this.message = format(message, ...args) }
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.
@climba03003 I implemented your solution; thank you.
@jsumners I tried your solution, but unfortunately it didn't pass tests.
I dislike the solution. |
@Uzlopak I appreciate the useful feedback. Closing and implementing my own solution that addresses our particular use case. Thank you for your assistance. |
Checklist
npm run test
andnpm run benchmark
and the Code of conduct
This PR proposes supporting empty strings as messages to support the following use case:
This supports the use case where the user doesn't desire any prefix or suffix to the message; they want the message to match the argument passed to the error.
The
trim()
I admit is a little problematic, but without it, the above example looks like: