IdoPesok / zsa

https://zsa.vercel.app
MIT License
447 stars 13 forks source link

Customise error format #83

Closed blechatellier closed 1 month ago

blechatellier commented 1 month ago

Hi @IdoPesok,

I'm throwing a ZSAError within a serverAction.

    if (!organization) {
      throw new ZSAError('NOT_FOUND', 'Organization not found');
    }

Can we customise the error payload please? I would like to exclude the stack trace and only return message and code. Seems a bit redundant to have both data and message with the same value.

{
    "data": "Organization not found",
    "name": "Error",
    "stack": "\"Error: Organization not found\\n    at $$ACTION_0 (webpack-internal:///(rsc)/./src/lib/actions/organizations/actions.ts:35:15)\\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\\n    at async wrapper (webpack-internal:///(rsc)/../../node_modules/.pnpm/zsa@0.3.0/node_modules/zsa/dist/index.mjs:393:22)\\n    at async getResponseFromAction (webpack-internal:///(rsc)/../../node_modules/.pnpm/zsa-openapi@0.0.8/node_modules/zsa-openapi/dist/index.mjs:2043:25)\\n    at async handler (webpack-internal:///(rsc)/../../node_modules/.pnpm/zsa-openapi@0.0.8/node_modules/zsa-openapi/dist/index.mjs:2149:12)\\n",
    "message": "Organization not found",
    "code": "NOT_FOUND"
}

Ideally I would like to have:

{
  "code": "NOT_FOUND",
  "message": "Organization not found"
}

or for a validation error:

{
  "code": "INPUT_PARSE_ERROR",
  "message": "Invalid uuid",
  "formattedErrors": { ... }
}

What do you think?

IdoPesok commented 1 month ago

84 adds a shape error option to the create route handlers : ) Will push this soon. Lmk if you have any feedback!

Screen Shot 2024-06-03 at 11 53 17 PM
IdoPesok commented 1 month ago

What do you think?

I think this is a great idea! Makes a lot of sense

blechatellier commented 1 month ago

@IdoPesok that looks good, sorry I updated the issue slightly as I thought maybe customising the error payload should be available globally no matter where it's called from, not just on the router. That said, my immediate need was to customise the payload for the OpenAPI so that definitely works.

IdoPesok commented 1 month ago

Good point. Possibly adding a shape error fn to actions and procedures?

const customErrorAction = createServerActionProcedure()
    .shapeError((error) => {
        return {
            code: error.code,
            message: error.message
        }
    })
    .handler(() => {})
    .createServerAction()

Then actions made from this procedure will inherit the shape error fn (so you don't need to rewrite it on every action). You would also be able to override/set shape error fn on actions themselves.

From looking at the code, will need to look into this further as it will require a refactor of error types. However, we can push this right now for routers as they don't have error types in their return.

blechatellier commented 1 month ago

I like it, it would be good if the original error could bubble up to this shapeError if it is defined so that the shape can be modified, if not, return the default shape you have.

If the original error is returned then I would be able to check the error type and use my own ApplicationError class instead of ZSAError.

And possibly even return a status field (ie: 400, 404) to customise the OpenAPI response code.

const customErrorAction = createServerActionProcedure()
    .shapeError((error) => {
      if (error instanceof ApplicationError) {
        return {
          code: error.code,
          message: error.message
        }
      }

      if (error instanceof ZodError) {
        return {
          code: 'VALIDATION_ERROR',
          message: error.message
        }
      }
    })
    .handler(() => {})
    .createServerAction()
blechatellier commented 1 month ago

@IdoPesok here is some code I use on another project to give an idea on how I handle errors,

import { ZodError } from 'zod';

export enum ErrorType {
  BadRequest = 'BadRequest',
  Unknown = 'Unknown',
  Unauthorized = 'Unauthorized',
  Conflict = 'Conflict',
  NotFound = 'NotFound',
}

export class ApplicationError extends Error {
  type: ErrorType;
  message: string;

  constructor(type: ErrorType = ErrorType.Unknown, message: string) {
    super(message);
    this.type = type;
    this.message = message;
  }
}

const ErrorStatus: Record<ErrorType, number> = {
  [ErrorType.BadRequest]: 400,
  [ErrorType.Unauthorized]: 401,
  [ErrorType.Unknown]: 500,
  [ErrorType.Conflict]: 409,
  [ErrorType.NotFound]: 404,
};

export const shapeError = (error: unknown) => {
  let errorType = ErrorType.Unknown;
  let issues;
  let message = 'Oops! Something went wrong';

  if (error instanceof ApplicationError) {
    errorType = error.type;
    message = error.message;
  }

  if (error instanceof ZodError) {
    errorType = ErrorType.BadRequest;
    issues = error.issues;
    message = 'Oops! Some of the data you provided is invalid';
  }

  const status = ErrorStatus[errorType];

  if (errorType === ErrorType.Unknown) {
    console.error(error);
  } else if (errorType === ErrorType.BadRequest) {
    console.warn(error);
  }

  return { type: errorType, issues, status, message };
};
IdoPesok commented 1 month ago

zsa-openapi@0.0.9 is up! Give it a shot, docs are here.

Concur bubbling up the original error. I will put this as high priority for the zsa as I think something like this would help improve error handling dramatically. Will link this thread once we get a PR up.

W.r.t. to status codes, the ZSA codes mapping to HTTP codes is here

blechatellier commented 1 month ago

Awesome thanks!