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

fix: cb is not a function at fallbackErrorHandler #5317

Merged
merged 2 commits into from Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/Reference/Errors.md
Expand Up @@ -92,6 +92,7 @@
- [FST_ERR_PLUGIN_NOT_PRESENT_IN_INSTANCE](#fst_err_plugin_not_present_in_instance)
- [FST_ERR_VALIDATION](#fst_err_validation)
- [FST_ERR_LISTEN_OPTIONS_INVALID](#fst_err_listen_options_invalid)
- [FST_ERR_ERROR_HANDLER_NOT_FN](#fst_err_error_handler_not_fn)

### Error Handling In Node.js
<a id="error-handling"></a>
Expand Down Expand Up @@ -361,4 +362,5 @@ but the body is being consumed. | Make sure you don't consume the `Response.body
| <a id="fst_err_plugin_not_present_in_instance">FST_ERR_PLUGIN_NOT_PRESENT_IN_INSTANCE</a> | The decorator is not present in the instance. | - | [#4554](https://github.com/fastify/fastify/pull/4554) |
| <a id="fst_err_validation">FST_ERR_VALIDATION</a> | The Request failed the payload validation. | Check the request payload. | [#4824](https://github.com/fastify/fastify/pull/4824) |
| <a id="fst_err_listen_options_invalid">FST_ERR_LISTEN_OPTIONS_INVALID</a> | Invalid listen options. | Check the listen options. | [#4886](https://github.com/fastify/fastify/pull/4886) |
| <a id="fst_err_error_handler_not_fn">FST_ERR_ERROR_HANDLER_NOT_FN</a> | Error Handler must be a function | Provide a function to `setErrorHandler`. | [#?](https://github.com/fastify/fastify/pull/?) |
Uzlopak marked this conversation as resolved.
Show resolved Hide resolved

7 changes: 6 additions & 1 deletion fastify.js
Expand Up @@ -71,7 +71,8 @@ const {
FST_ERR_INSTANCE_ALREADY_LISTENING,
FST_ERR_REOPENED_CLOSE_SERVER,
FST_ERR_ROUTE_REWRITE_NOT_STR,
FST_ERR_SCHEMA_ERROR_FORMATTER_NOT_FN
FST_ERR_SCHEMA_ERROR_FORMATTER_NOT_FN,
FST_ERR_ERROR_HANDLER_NOT_FN
} = errorCodes

const { buildErrorHandler } = require('./lib/error-handler.js')
Expand Down Expand Up @@ -844,6 +845,10 @@ function fastify (options) {
function setErrorHandler (func) {
throwIfAlreadyStarted('Cannot call "setErrorHandler"!')

if (typeof func !== 'function') {
throw new FST_ERR_ERROR_HANDLER_NOT_FN()
}

this[kErrorHandler] = buildErrorHandler(this[kErrorHandler], func.bind(this))
return this
}
Expand Down
6 changes: 6 additions & 0 deletions lib/errors.js
Expand Up @@ -64,6 +64,12 @@ const codes = {
500,
TypeError
),
FST_ERR_ERROR_HANDLER_NOT_FN: createError(
'FST_ERR_ERROR_HANDLER_NOT_FN',
'Error Handler must be a function',
500,
TypeError
),

/**
* ContentTypeParser
Expand Down
24 changes: 17 additions & 7 deletions test/internals/errors.test.js
Expand Up @@ -5,7 +5,7 @@ const errors = require('../../lib/errors')
const { readFileSync } = require('node:fs')
const { resolve } = require('node:path')

test('should expose 79 errors', t => {
test('should expose 80 errors', t => {
t.plan(1)
const exportedKeys = Object.keys(errors)
let counter = 0
Expand All @@ -14,11 +14,11 @@ test('should expose 79 errors', t => {
counter++
}
}
t.equal(counter, 79)
t.equal(counter, 80)
})

test('ensure name and codes of Errors are identical', t => {
t.plan(79)
t.plan(80)
const exportedKeys = Object.keys(errors)
for (const key of exportedKeys) {
if (errors[key].name === 'FastifyError') {
Expand Down Expand Up @@ -827,8 +827,18 @@ test('FST_ERR_LISTEN_OPTIONS_INVALID', t => {
t.ok(error instanceof TypeError)
})

test('FST_ERR_ERROR_HANDLER_NOT_FN', t => {
t.plan(5)
const error = new errors.FST_ERR_ERROR_HANDLER_NOT_FN()
t.equal(error.name, 'FastifyError')
t.equal(error.code, 'FST_ERR_ERROR_HANDLER_NOT_FN')
t.equal(error.message, 'Error Handler must be a function')
t.equal(error.statusCode, 500)
t.ok(error instanceof TypeError)
})

test('Ensure that all errors are in Errors.md TOC', t => {
t.plan(79)
t.plan(80)
const errorsMd = readFileSync(resolve(__dirname, '../../docs/Reference/Errors.md'), 'utf8')

const exportedKeys = Object.keys(errors)
Expand All @@ -840,7 +850,7 @@ test('Ensure that all errors are in Errors.md TOC', t => {
})

test('Ensure that non-existing errors are not in Errors.md TOC', t => {
t.plan(79)
t.plan(80)
const errorsMd = readFileSync(resolve(__dirname, '../../docs/Reference/Errors.md'), 'utf8')

const matchRE = / {4}- \[([A-Z0-9_]+)\]\(#[a-z0-9_]+\)/g
Expand All @@ -853,7 +863,7 @@ test('Ensure that non-existing errors are not in Errors.md TOC', t => {
})

test('Ensure that all errors are in Errors.md documented', t => {
t.plan(79)
t.plan(80)
const errorsMd = readFileSync(resolve(__dirname, '../../docs/Reference/Errors.md'), 'utf8')

const exportedKeys = Object.keys(errors)
Expand All @@ -865,7 +875,7 @@ test('Ensure that all errors are in Errors.md documented', t => {
})

test('Ensure that non-existing errors are not in Errors.md documented', t => {
t.plan(79)
t.plan(80)
const errorsMd = readFileSync(resolve(__dirname, '../../docs/Reference/Errors.md'), 'utf8')

const matchRE = /<a id="[0-9a-zA-Z_]+">([0-9a-zA-Z_]+)<\/a>/g
Expand Down
13 changes: 13 additions & 0 deletions test/set-error-handler.test.js
@@ -0,0 +1,13 @@
'use strict'

const t = require('tap')
const test = t.test
const Fastify = require('..')
const { FST_ERR_ERROR_HANDLER_NOT_FN } = require('../lib/errors')

test('setErrorHandler should throw an error if the handler is not a function', t => {
t.plan(1)
const fastify = Fastify()

t.throws(() => fastify.setErrorHandler('not a function'), new FST_ERR_ERROR_HANDLER_NOT_FN())
})