Refactor errors to use @fastify/error and throw correct validation error

[Bram-dc]

This pull request changed the plugin to use the internal errors from fastify and also return the correct validation errors.

See official type provider: https://github.com/fastify/fastify-type-provider-typebox/blob/main/index.mts

    return {
      // Note: Here we return a FastifySchemaValidationError[] result. As of writing, Fastify
      // does not currently export this type. Future revisions should uncomment the assertion
      // below and return the full set of properties. The specified properties 'message' and
      // 'instancePath' do however result in a near equivalent error messages to Ajv.
      error: errors.map((error) => ({
        message: `${error.message}`,
        instancePath: error.path
      })) // as FastifySchemaValidationError[]
    }

• Move ResponseValidationError class to errors.ts
• Add InvalidSchemaError class to errors.ts
• Update imports in core.ts to reflect new file locations
• Update resolveSchema function in core.ts to throw InvalidSchemaError

[kibertoad]

why can't we return details here and assert that validation error exposes enough details?

[Bram-dc]

There is no details on ResponseSerializationError.

We would now have to access it through error.cause.

edited since I stated it wrong

[kibertoad]

that's fine, can we use that field to preserve the initial test logic?

[Bram-dc]

Done, I also made a mistake so I renamed ResponseValidationError to ResponseSerializationError since that is what it is.

[Bram-dc]

Validation errors can be accessed through error.validation as described in the docs.

https://fastify.dev/docs/latest/Reference/Validation-and-Serialization/#error-handling

fastify.setErrorHandler(function (error, request, reply) {
  if (error.validation) {
     reply.status(422).send(new Error('validation failed'))
  }
})

[kibertoad]

fastify error model really isn't very flexible :-/
trying to augment errors we are getting there currently

[kibertoad]

this is less useful than what we had before.

does the new error structure no longer expose the url and the method?

[Bram-dc]

Well we could also throw the old error here, but it is mostly for consistency. This type of error should also result in a 500 Internal server error, so I think it is not important to add the url and method to the error. The default error reporting from fastify reports these values anyways with the error.

[Bram-dc]

Plus you can just get those values from the error handler.

[kibertoad]

will need to check it by hand in greater detail to understand whether this is sufficient.

I am primarily concerned about getting good bugsnag entries and logs, i n c l u d i n g on the client side.

whatever solution we end up with, it should expose no less data than the current one.

[kibertoad]

@Bram-dc I've made some adjustments, can you please check if you are OK with them?

[Bram-dc]

Why is this not an arrow function? All others are using an arrow function.

[kibertoad]

only the ones that are exported

[Bram-dc]

Ah okay

[Bram-dc]

FastifySchemaValidationError is a different error and should not be used here.

export interface FastifySchemaValidationError {
  keyword: string;
  instancePath: string;
  schemaPath: string;
  params: Record<string, unknown>;
  message?: string;
}

[Bram-dc]

Neither here

[Bram-dc]

Can I force push an alternative?

[kibertoad]

@Bram-dc sure! as long as it preserves the expectations of the test

[kibertoad]

this is perfect, thank you!