cloudflare / chanfana

OpenAPI 3 and 3.1 schema generator and validator for Hono, itty-router and more!
https://chanfana.pages.dev
MIT License
302 stars 40 forks source link

Global error handling / customize response json format #165

Closed marceloverdijk closed 2 months ago

marceloverdijk commented 2 months ago

In case of a zod validation error the error response is something like:

{
    "errors": [
        {
            "code": "invalid_type",
            "expected": "string",
            "received": "array",
            "path": [
                "query",
                "sort"
            ],
            "message": "Expected string, received array"
        }
    ],
    "success": false,
    "result": {}
}

I want to change this error format globally (for all routers) so it is uses the JSON:API specification for errors:

{
  "errors": [
    {
      "status": "422",
      "title":  "invalid_type",
      "detail": "Expected string, received array."
      "meta": {
            "code": "invalid_type",
            "expected": "string",
            "received": "array",
            "path": [
                "query",
                "sort"
            ],
            "message": "Expected string, received array"
      },
    }
  ]
}

What is the best way to solve this?

Besides zod errors, my routers may call service class that throw errors as well. I want to handle the globally, and create a similar response. I'm missing any relevant information about how to do such (global) error handling.

marceloverdijk commented 2 months ago

I've used the itty-router catch option to configure a custom error handler like:

const router = Router({
  catch: errorHandler,
});

this works for custom exception/error thrown from my services classes.

But, this does not allow me to customize the zod validation error responses unfortunately. @G4brym any recommendations on this?

marceloverdijk commented 2 months ago

PS: I went now for this quick hack'ish solution:

OpenAPIRoute.prototype.handleValidationError = function(errors: z.ZodIssue[]): Response {
  throw new z.ZodError(errors);
};

const router = Router({
  catch: errorHandler,
});

and in that error handler, besides my custom errors, I also handle the zod errors to render the expected error format for my application.

I think it would nice if Chanfana would provide a custom handleValidationError router option to customize the zod validation error responses.

G4brym commented 2 months ago

Hey @marceloverdijk I just wrote a quick page on how to handle validation errors across multiple endpoints. I think making use of JS classes is better than defining a global error handler in the router, this way the developer can have multiple error handlers and choose to extend just the desired endpoints let me know what you think https://chanfana.pages.dev/advanced-user-guide/custom-error-handling/

marceloverdijk commented 2 months ago

Thx, yes extending OpenAPIRoute was my other option. Thx for sharing!

marceloverdijk commented 2 months ago

Hi @G4brym ,

While extending OpenAPIRoute and looking at the route.ts source I noticed:

  getSchema(): OpenAPIRouteSchema {
    // Use this function to overwrite schema properties
    return this.schema
  }

Which gave me some idea ;-)

As I have a fair amount of endpoints having paging params (page and page_size) I thought I could maybe create a MyPagingRoute and add these params to the schema automatically. And define a applyQueryParams to this class to call it from handle implementations.

However, I noticed these paging params will then not be part of the the validated schema data:

const data = await this.getValidatedData<typeof this.schema>();
const page = data.query.page // or data.query[QueryParam.PAGE];

So maybe my approach is not recommended at all ;-) I'm wondering if you are using similar patterns like this at Cloudflare or not?

My alternative is to simply abandon my MyPagingRoute idea and have some re-usable shema objects directly in the OpenAPIRouteSchema.schema definition.

export class MyPagingRoute extends MyProjectRoute {
  defaultPageSize = 10;
  maxPageSize = 50;

  getSchema(): OpenAPIRouteSchema {
    extendQuerySchema(this.schema, QueryParam.PAGE, z.number().int().optional());
    extendQuerySchema(this.schema, QueryParam.PAGE_SIZE, z.number().int().max(this.maxPageSize).optional());
    return super.getSchema();
  }

  applyQueryParams(qb: SQLiteSelect): void {
    // get page and page_size from validated data or directly from raw request?
    // how to do this best?
  }
}

const extendQuerySchema = (schema: OpenAPIRouteSchema, field: QueryParam, augmentation: ZodTypeAny) => {
  // Ensure request and request.query schema are defined.
  schema.request = schema.request || {};
  schema.request.query = schema.request.query || z.object({});

  const queryShape = schema.request.query.shape;

  // Extend schema (only if field is not present).
  if (!(field in queryShape)) {
    schema.request.query = schema.request.query.extend({ [field]: augmentation });
  }
};
G4brym commented 2 months ago

I have a bunch of these kinds of parameter injections, like removing debug parameters in production envs, etc You need to overwrite the getSchemaZod function, this way the new parameters will be part of the validation schema.

This example should work, you can also safely call multiple times the this.getValidatedData() as it will only be executed once

export class MyPagingRoute extends MyProjectRoute {
  defaultPageSize = 10;
  maxPageSize = 50;

  getSchemaZod(): OpenAPIRouteSchema {
    // Deep copy
    const schema = { ...this.getSchema() }

   schema.request.query = schema.request.query.merge(z.object({
     page: z.number().int().optional()
     page_size: z.number().int().max(this.maxPageSize).optional()
   }))

    // @ts-ignore
    return schema
  }

  applyQueryParams(qb: SQLiteSelect): void {
     const validatedData = await this.getValidatedData()

    // do something with page and size
  }
}

Edit: Actually, overwriting the getSchema should also have worked, maybe it didn't because you tried to update this.schema direcly without doing a deep copy of the object

marceloverdijk commented 2 months ago

Hi @G4brym thx for your reply. I tried with both overriding getSchemaZod() and getSchema().

And it gets the data with the following code:

image

but it complains query could be undefined.

image

so this way I seem to lose all type safety... is that expected with this approach?

marceloverdijk commented 2 months ago

@G4brym please correct me if wrong. I think for removing parameters overriding the getSchema/getSchemaZod is useful. But for adding fields it might not be useful. E.g. augmenting the schema with the page and page:size query parameters in the MyPagingRoute does not allow me to type safely get those parameters in the router itself (via this.getValidatedData<typeof this.schema>() as this.schema does not have those paramters).