refactor: use @fastify/error
#123
Conversation
test/index.test.js
Outdated
| t.assert.equal(errorHandler.mock.callCount(), 1) | ||
| t.assert.strictEqual(errorHandler.mock.calls[0].error, undefined, 'errorHandler function has emitted an unexpected error') |
There was a problem hiding this comment.
I want it to be as close as possible to how users are supposed to use this utility function, but I’m not sure this is the best way to test this case.
index.js
Outdated
| const NoSchemaDefinedError = createError('FST_NO_SCHEMA_DEFINED_ERROR', 'No schema defined for %s') | ||
|
|
||
| function isResponseValidationError (error) { | ||
| return error ? error.code === FST_RESPONSE_VALIDATION_ERROR : false |
There was a problem hiding this comment.
This seems to me like a reasonable way to identify the nature of the error.
JacopoPatroclo
changed the title
@fastify/error@fastify/error
index.js
Outdated
| const ReplyValidationFailError = createError(FST_RESPONSE_VALIDATION_ERROR, '%s', 500) | ||
| const NoSchemaDefinedError = createError('FST_NO_SCHEMA_DEFINED_ERROR', 'No schema defined for %s') | ||
|
|
||
| function isResponseValidationError (error) { |
There was a problem hiding this comment.
I'd suggest reduce the scope of this PR and evaluate the implementation of such utility in another PR.
example.mjs
Outdated
| app.setErrorHandler((error, _request, reply) => { | ||
| if (isResponseValidationError(error)) { | ||
| return reply.status(500).send({ ok: false }) | ||
| } | ||
| reply.status(200).send({ ok: true }) | ||
| }) | ||
|
|
There was a problem hiding this comment.
Beside my suggestion of implementing isResponseValidationError with another PR, I would not provide this example since a lot of users might copy paste it and assume it's a mandatory step to register the plugin.
index.js
Outdated
| const FST_RESPONSE_VALIDATION_ERROR = 'FST_RESPONSE_VALIDATION_ERROR' | ||
|
|
||
| const ReplyValidationFailError = createError(FST_RESPONSE_VALIDATION_ERROR, '%s', 500) | ||
| const NoSchemaDefinedError = createError('FST_NO_SCHEMA_DEFINED_ERROR', 'No schema defined for %s') |
There was a problem hiding this comment.
I'd suggest to prefix all errors with FST_RESPONSE_VALIDATION.
This is just a naming suggestion to support my point:
FST_RESPONSE_VALIDATION_FAILED_VALIDATIONFST_RESPONSE_VALIDATION_SCHEMA_NOT_DEFINED
JacopoPatroclo
force-pushed
the
master
branch
from
c2f3f0f to
a7fca0a
Compare
I agree. I'll open a new PR with this feature if @mcollina is fine with exposing a new utility function for it. |
The aim of this PR is to address the issue #121.
I’ve migrated the errors emitted by this plugin to use
@fastify/error, which has enabled the creation of a utility function to check if the error is actually coming from this plugin by using the error code. This makes it easier for consumers to implement specific error-handling logic around it.Checklist
npm run testandnpm run benchmarkand the Code of conduct