should this package expose FastifyError?

[ahmadnassri]

🚀 Feature Proposal

should export FastifyError function so it can be used with err instanceOf FastifyError in plain JavaScript

is there a reason this is not currently exposed? seems like the use-case is primarily focused on TypeScript, any reason we shouldn't have access to it for plain JS.

Motivation

in a Fastify errorHandler, you want to intercept errors and check what they are

Example

const { FastifyError } = require('fastify-error')
const { DatabaseError } = require('pg-protocol/dist/messages')

module.exports = function (error, request, reply) {
    // write to log
    request.log.fatal({ ...error }, error.message)

    if (error instanceOf FastifyError) {
        // do stuff
    }

    if (error instanceof SyntaxError) {
        // do other stuff
    }

    if (error.validation) {
        // handle input validation errors
    }

    if (error instanceof DatabaseError) {
        // replace db query errors with public friendly ones
    }
  }
}

[mcollina]

My recommendation is to never use instanceof for these types of checks. Use the error codes, they are much more predictable.
(As an example, Jest messes up with the global Error object, making this code incredibly hard to test).

I can see why a lot of people would like to support this pattern. The way this is implemented it does not have a generic FastifyError class. It's instantiated dynamically every time createError is called, and we support passing through a Base class exactly for this purpose. I think in Fastify we could export a new base class for all our Fastify errors.

[ahmadnassri]

As an example, Jest messes up with the global Error object, making this code incredibly hard to test

indeed, which is why I never use Jest 😉

I think in Fastify we could export a new base class for all our Fastify errors.

whichever class is exported, can then be used later to identify the error instance, since it will have to be import-ed/require-ed directly ...

[mcollina]

whichever class is exported, can then be used later to identify the error instance, since it will have to be import-ed/require-ed directly ...

Yes exactly! Would you like to send a PR to fastify?

[xtx1130]

Well, because of inherits(FastifyError, Base). We must change the Base params to generic FastifyError class. And I think it will make breaking change for fastify:

FST_ERR_CTP_EMPTY_TYPE: createError(
    'FST_ERR_CTP_EMPTY_TYPE',
    'The content type cannot be an empty string',
    500,
    TypeError // We need to change this params to generic `FastifyError` class and we can't pass `TypeError` anymore
  )

[fox1t]

What about the Boom approach: adding isFastifyError to the instances and a static method that checks if an object is a FastifyError instance?
Reference: https://hapi.dev/module/boom/api/?v=10.0.0#isboomerr-statuscode

[mcollina]

That would work for me.

[jsumners]

Do not make it an instanceof check. Add a Symbol.toStringTag and check Object.prototype.toString.call.

[fox1t]

Do not make it an instanceof check. Add a Symbol.toStringTag and check Object.prototype.toString.call.

I am not sure how this plays out with my proposal. Is this an alternative approach?

[jsumners]

function isFastifyError(obj) {
  return Object.prototype.toString.call(obj) === '[object FastifyError]'
}

The instanceof operator breaks when different versions of the package are present in a project.

[fox1t]

Yes. That's why I don't want to use it either. I am firmly against the instanceof operator. I prefer Boom's approach for that reason. It is better to ship the check function directly inside this lib from the DX perspective.

[jsumners]

I prefer Boom's approach for that reason..

[fox1t]

Sorry for phrasing it badly. What I meant is to expose a function that will check our own known property. 👌

[Uzlopak]

What i wrote in the corresponding PR:
The question is if we should instead improve the documentation what we consider best practise and about ducktyping FastifyError.

I could do that instead, if we agree on

[piotr-cz]

So, what is the recommended way to check if error is a FastifyError?

I'm implementing optional JWT authentication with @fastify/jwt like so:

fastify.decorate<onRequestAsyncHookHandler>('authenticateUserOptional', async function (request) {
  try {
    await request.userJwtVerify()
  } catch (error: unknown) {
    // Is it the recommended way?

    // Ignore when token is missing
    if (error instanceof Error && 'code' in error && error.code === 'FST_JWT_BAD_REQUEST') {
      return
    }

    throw error
  }
})

[bradennapier]

This feels like a terrible recommended way to handle errors - and the documentation method does not allow us to easily handle all error cases without a bunch of if/else statements using instanceof ?

code is not even in the FastifyConstructor type so you have to do the 'code' in error check which makes no sense..

I find myself having to do this 
 

 if (error.code && error.code.startsWith('FST')) {
        logType = 'critical';
        const result = handleFastifyError(error);
      }

To check if its a fastify error but then if i try to do :

function handleFastifyError(error: FastifyError) {
  switch (error.code) {
    case errorCodes.FST_ERR_ROUTE_METHOD_INVALID:
    case errorCodes.FST_ERR_ROUTE_METHOD_NOT_SUPPORTED: {
      break;
    }
    case errorCodes.FST_ERR_CTP_INVALID_TYPE:
    case errorCodes.FST_ERR_CTP_EMPTY_TYPE:
    case errorCodes.FST_ERR_MISSING_CONTENTTYPE_SERIALIZATION_FN:
    case errorCodes.FST_ERR_CTP_INVALID_MEDIA_TYPE: {
      break;
    }
    case errorCodes.FST_ERR_CTP_BODY_TOO_LARGE: {
      break;
    }
    case errorCodes.FST_ERR_NOT_FOUND: {
      return 
      break;
    }

    default:
      break;
  }

}

you cant do `case errorCodes.FST_ERR_ROUTE_METHOD_INVALID.code because code is not in the type

• At the very least, code should be a static member on the errors for this purpose
• An enum could be provided, although less ideal

Using a string comparison directly is not ideal as it doesn't protect against upstream code changes in future unless FastifyError.code was a strict union which is defined in the fastify module anyway:

export type FastifyErrorCodes = Record<
'FST_ERR_NOT_FOUND' |
'FST_ERR_OPTIONS_NOT_OBJ' |
'FST_ERR_QSP_NOT_FN' |
'FST_ERR_SCHEMA_CONTROLLER_BUCKET_OPT_NOT_FN' |
'FST_ERR_SCHEMA_ERROR_FORMATTER_NOT_FN' |
'FST_ERR_AJV_CUSTOM_OPTIONS_OPT_NOT_OBJ' |
'FST_ERR_AJV_CUSTOM_OPTIONS_OPT_NOT_ARR' |
'FST_ERR_VERSION_CONSTRAINT_NOT_STR' |
'FST_ERR_VALIDATION' |
'FST_ERR_CTP_ALREADY_PRESENT' |
'FST_ERR_CTP_INVALID_TYPE' |
'FST_ERR_CTP_EMPTY_TYPE' |
'FST_ERR_CTP_INVALID_HANDLER' |
'FST_ERR_CTP_INVALID_PARSE_TYPE' |
'FST_ERR_CTP_BODY_TOO_LARGE' |
'FST_ERR_CTP_INVALID_MEDIA_TYPE' |
'FST_ERR_CTP_INVALID_CONTENT_LENGTH' |
'FST_ERR_CTP_EMPTY_JSON_BODY' |
'FST_ERR_CTP_INSTANCE_ALREADY_STARTED' |
'FST_ERR_DEC_ALREADY_PRESENT' |
'FST_ERR_DEC_DEPENDENCY_INVALID_TYPE' |
'FST_ERR_DEC_MISSING_DEPENDENCY' |
'FST_ERR_DEC_AFTER_START' |
'FST_ERR_HOOK_INVALID_TYPE' |
'FST_ERR_HOOK_INVALID_HANDLER' |
'FST_ERR_HOOK_INVALID_ASYNC_HANDLER' |
'FST_ERR_HOOK_NOT_SUPPORTED' |
'FST_ERR_MISSING_MIDDLEWARE' |
'FST_ERR_HOOK_TIMEOUT' |
'FST_ERR_LOG_INVALID_DESTINATION' |
'FST_ERR_LOG_INVALID_LOGGER' |
'FST_ERR_REP_INVALID_PAYLOAD_TYPE' |
'FST_ERR_REP_ALREADY_SENT' |
'FST_ERR_REP_SENT_VALUE' |
'FST_ERR_SEND_INSIDE_ONERR' |
'FST_ERR_SEND_UNDEFINED_ERR' |
'FST_ERR_BAD_STATUS_CODE' |
'FST_ERR_BAD_TRAILER_NAME' |
'FST_ERR_BAD_TRAILER_VALUE' |
'FST_ERR_FAILED_ERROR_SERIALIZATION' |
'FST_ERR_MISSING_SERIALIZATION_FN' |
'FST_ERR_MISSING_CONTENTTYPE_SERIALIZATION_FN' |
'FST_ERR_REQ_INVALID_VALIDATION_INVOCATION' |
'FST_ERR_SCH_MISSING_ID' |
'FST_ERR_SCH_ALREADY_PRESENT' |
'FST_ERR_SCH_CONTENT_MISSING_SCHEMA' |
'FST_ERR_SCH_DUPLICATE' |
'FST_ERR_SCH_VALIDATION_BUILD' |
'FST_ERR_SCH_SERIALIZATION_BUILD' |
'FST_ERR_SCH_RESPONSE_SCHEMA_NOT_NESTED_2XX' |
'FST_ERR_HTTP2_INVALID_VERSION' |
'FST_ERR_INIT_OPTS_INVALID' |
'FST_ERR_FORCE_CLOSE_CONNECTIONS_IDLE_NOT_AVAILABLE' |
'FST_ERR_DUPLICATED_ROUTE' |
'FST_ERR_BAD_URL' |
'FST_ERR_ASYNC_CONSTRAINT' |
'FST_ERR_DEFAULT_ROUTE_INVALID_TYPE' |
'FST_ERR_INVALID_URL' |
'FST_ERR_ROUTE_OPTIONS_NOT_OBJ' |
'FST_ERR_ROUTE_DUPLICATED_HANDLER' |
'FST_ERR_ROUTE_HANDLER_NOT_FN' |
'FST_ERR_ROUTE_MISSING_HANDLER' |
'FST_ERR_ROUTE_METHOD_INVALID' |
'FST_ERR_ROUTE_METHOD_NOT_SUPPORTED' |
'FST_ERR_ROUTE_BODY_VALIDATION_SCHEMA_NOT_SUPPORTED' |
'FST_ERR_ROUTE_BODY_LIMIT_OPTION_NOT_INT' |
'FST_ERR_ROUTE_REWRITE_NOT_STR' |
'FST_ERR_REOPENED_CLOSE_SERVER' |
'FST_ERR_REOPENED_SERVER' |
'FST_ERR_INSTANCE_ALREADY_LISTENING' |
'FST_ERR_PLUGIN_VERSION_MISMATCH' |
'FST_ERR_PLUGIN_NOT_PRESENT_IN_INSTANCE' |
'FST_ERR_PLUGIN_CALLBACK_NOT_FN' |
'FST_ERR_PLUGIN_NOT_VALID' |
'FST_ERR_ROOT_PLG_BOOTED' |
'FST_ERR_PARENT_PLUGIN_BOOTED' |
'FST_ERR_PLUGIN_TIMEOUT'
, FastifyErrorConstructor>