fix: handle non FastifyErrors in custom handler properly, set type of error-parameter for setErrorHandler and errorHandler to unknown, but configurable via generic TError by Uzlopak · Pull Request #6308 · fastify/fastify

Uzlopak · 2025-09-05T11:43:01Z

Change in TypeScript typings for `setErrorHandler` and `errorHandler`

The TypeScript typings of setErrorHandler and errorHandler have changed.
The error parameter is now typed as unknown.

Previously, it was assumed that errorHandler would always receive a FastifyError. However, this assumption is unsafe because JavaScript allows throwing any value, not just Error objects. For example:

throw null

This value could be passed into the errorHandler. If your code then tries to access properties like error.code while assuming a FastifyError, it may result in runtime errors.

To make this explicit, the error parameter is now typed as unknown.

Example

import Fastify from 'fastify'
// FastifyError is only exported as a type from fastify
// Import it from @fastify/error for runtime checks with instanceof
// Make sure version 4.2.0 or higher is used across your dependency tree
import { FastifyError } from '@fastify/error'

const server = Fastify()

// TypeScript will now report an error because `error` is `unknown`
server.setErrorHandler(function (error, request, reply) {
  if (error.code === 'FST_ERR_TEST') { // ❌ TS18046: 'error' is of type 'unknown'
    // do stuff
  }
})

Workarounds

You can force the old behavior by typing error yourself, but this bypasses the safety of the new typings:

Using a generic (Not Recommended)

server.setErrorHandler<FastifyError>(function (error, request, reply) {
  if (error.code === 'FST_ERR_TEST') {
    // do stuff
  }
})

Typing the parameter directly (Not Recommended)

server.setErrorHandler(function (error: FastifyError, request, reply) {
  if (error.code === 'FST_ERR_TEST') {
    // do stuff
  }
})

Type guard with property checks

function isFastifyError(error: unknown): error is FastifyError {
  return (
    error !== null &&
    typeof error === 'object' &&
    typeof (error as Record<string, any>).code === 'string' &&
    typeof (error as Record<string, any>).statusCode === 'number' &&
    typeof (error as Record<string, any>).message === 'string'
  )
}

server.setErrorHandler(function (error, request, reply) {
  if (isFastifyError(error)) {
    if (error.code === 'FST_ERR_TEST') {
      // do stuff
    }
  } else {
    // Handle non-FastifyError values safely
    reply.code(500).send({
      message: 'Internal Server Error',
      error: 'Internal Server Error',
      statusCode: 500
    })
  }
})

⚠️ Do not rely on checking whether the code property starts with 'FST_' to determine if a value is a FastifyError.

Recommended Approach

Use an instanceof check to ensure error is a FastifyError.
This approach is safer and avoids assumptions:

server.setErrorHandler(function (error, request, reply) {
  if (error instanceof FastifyError) {
    if (error.code === 'FST_ERR_TEST') {
      // do stuff
    }
  } else {
    // Handle other error shapes safely
    reply.code(500).send({
      message: 'Internal Server Error',
      error: 'Internal Server Error',
      statusCode: 500
    })
  }
})

⚠️ Ensure that your project (and all dependencies) use @fastify/error version ≥ 4.2.0 to guarantee instanceof works reliably.

gurgunday

lgtm

makes sense

types/instance.d.ts

paulius-valiunas · 2025-09-24T10:49:27Z

Also, if (error.code === 'FST_ERR_WHATEVER') { is already invalid code, except your incorrect type definitions lead the developer to think it's valid. But it's not, because there's a lot of cases where writing completely valid Typescript would break it at runtime. I understand that you don't like when people throw non-Error objects, and neither do I. But like it or not, it's 100% valid Typescript, you don't need to break out of the type system and do dynamic casts or write pure Javascript to do it.

Furthermore, even if you only throw Error objects, they still don't have a code property by default. That is something your specific FastifyError type adds, and the expectation that everyone will wrap everything and ONLY throw your error type from their route handlers is neither reasonable nor realistic.

paulius-valiunas · 2025-09-24T11:22:46Z

I understand your arguments, but in practice you are the only one that raised the issue in years as far as I know. So do you really think there is an urgency to publish a code that is gonna generate errors in thousands of perfectly fine code bases in the next minor?

I think most people using your package are unaware of the risk they're taking by assuming the types are correct. No, those codebases are NOT "perfectly fine", in fact they are not fine at all because they're built incorrectly.

jean-michelet · 2025-09-24T11:51:06Z

in fact they are not fine at all because they're built incorrectly.

That looks like a very exaggerated statement to me.
Maybe in some cases, mistaking Error for FastifyError could cause developers to make mistakes that could lead to issues based on an unfortunate condition.
But this not obvious at all, and they would probably notice it.

Anyway, I understand the safety concerns around misleading typing.
I am not radically opposed to a breaking change, even though I think this one is big.

@Eomm @mcollina
Can you plz clarify if we offer semver guarantees on types?
Do you agree with changing type of the error to unknown in error handler?

paulius-valiunas · 2025-09-24T12:41:22Z

in fact they are not fine at all because they're built incorrectly.

That looks like a very exaggerated statement to me. Maybe in some cases, mistaking Error for FastifyError could cause developers to make mistakes that could lead to issues based on an unfortunate condition.

Hold on, it's not just "some cases" of "mistaking" one type for another. It's all cases where you trust the type definitions to correctly tell you what type you're going to receive. Currently, it says you will receive FastifyError, which is objectively incorrect.

But this not obvious at all, and they would probably notice it.

How exactly are Typescript developers supposed to "notice" that not all objects received in their handler have a code property, when the compiler tells them they all do?

I'm sorry, but I can't agree with what you're saying. It's absolutely obvious that the type definition is wrong and Typescript will do absolutely nothing to warn the developer about these hidden runtime errors.

mcollina · 2025-09-24T19:18:16Z

TypeScript and DefinitelyTyped do not offer any semver guarantees on anything. The way most breakages are handled in DefinitelyTyped is by patching all the dependents.

You can read a good explanation here:
https://www.learningtypescript.com/articles/why-typescript-doesnt-follow-strict-semantic-versioning. In the past we had to break the types simply because it was impossible to not do so if we did not want to “fall behind” the TypeScript ecosystem.

Even for pure-TS packages, you can’t assume types are correct in any form.

As a generic rule, if a type is incorrect and could lead to user bugs, I’m ok to change it (it’s a bug after all!) even if it causes downstream build errors.

How big of a breakage are we looking at? Can somebody make a few examples?

jean-michelet · 2025-09-24T20:19:06Z

TypeScript and DefinitelyTyped do not offer any semver guarantees on anything. The way most breakages are handled in DefinitelyTyped is by patching all the dependents.

I must shamefully admit that I was unaware of this.
So, let's go for unknown!

Uzlopak · 2025-09-24T21:19:45Z

@mcollina

My assessment is:
We have to expect that in every fastify implementation a setErrorHandler() and/or errorHandler() call exists. This means, that every product will be affected.

I disagree with @paulius-valiunas opinion, that we should not allow typing error as Generics. Atleast not without atleast one additional utility function provided by us.

If we have generics, then the developer will see that there is some potential issue with his setup.

e.g.

before:

setErrorHandler(function (err, req, reply) {
  if (err.code === 'FST_ERR_DOH') {
    // everything fine
  }
})

setting err to unknown

setErrorHandler(function (err, req, reply) {
  if (err.code === 'FST_ERR_DOH') {
    // err is unknown, no guarantee  that it is an object with a code attribute. E.g. if it is for some reason
    // null (is this even possible?)
  }
})

if it is a generic, than a developer can decide that he simply want to ignore it like before

setErrorHandler<FastifyError>(function (err, req, reply) {
  if (err.code === 'FST_ERR_DOH') {
    // everything is fine again
  }
})

So it would take about 5 minutes to figure out how to restore the old behaviour.

If we dont make it possible to pass FastifyError as generic, than we should consider to actually think how to solve it properly. Just nagging and to deny to configure the type of the error parameter without giving the right tools to the hand, means that a dev now has to find a "proper" solution.

We could actually ensure that we always have a FastifyError. We just need to patch handleError in error-handler.js

In the following lines we pass the received error:

fastify/lib/error-handler.js

Line 64 in 6a428d9

fallbackErrorHandler(error, reply, cb)

fastify/lib/error-handler.js

Line 69 in 6a428d9

const result = func(error, reply.request, reply)

So we could check if error is of type FastifyError, and if not instantiate a new special FastifyError without stacktraces, where we assign the provided value as cause attribute. We could even patch an native Error to be shaped like a FastifyError.

Maybe this is something we should implement in fastify-errors. A mutateToFastifyError() (name should be better) function, where you can pass anything and get a valid FastifyError.

It makes sense to consider also the actual runtime issues, if you use the default error handler. Will we accidently expose critical information?

throw 'the password is abc'

@jean-michelet

Well, Typescript is basically breaking on every minor. It would be more honest if they would just had versioned it typescript v. 59.2 and not 5.9.2.

add generic to errorHandler

Uzlopak · 2025-09-25T12:43:00Z

I marked some comments as outdated or off-topic to reduce the amount of text needed to read. I kept those, which have relevant content like technical arguments and final decisions and which are not containig repetitive arguments.

I wrote a text into the first post, to be potentially used for the next release to give the devs some approaches to fix this properly.

I also investigated our code and found a bug, that when null is thrown the default error handler threw itself an error when trying to access the statusCode attribute from the supposed error object.

gurgunday

Having a generic here achieves 2 things:

Allows folks who don't want to deal with the type change right away to easily fix compilation errors
Makes typing easier in projects where only specific errors are thrown at runtime

These are OK reasons to have a generic that defaults to unknown. But I know for a fact that we will get many issues because of this, so we do need to present it as a notable change

mcollina

lgtm

types: loosen setErrorHandler and errorHandler types

ca255f4

github-actions bot added the typescript TypeScript related label Sep 5, 2025

gurgunday approved these changes Sep 5, 2025

View reviewed changes

paulius-valiunas reviewed Sep 8, 2025

View reviewed changes

types/instance.d.ts Outdated Show resolved Hide resolved

paulius-valiunas reviewed Sep 8, 2025

View reviewed changes

types/instance.d.ts Outdated Show resolved Hide resolved