Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for custom error catcher for api #1812

Closed
nechaido opened this issue Feb 26, 2023 · 10 comments
Closed

Support for custom error catcher for api #1812

nechaido opened this issue Feb 26, 2023 · 10 comments

Comments

@nechaido
Copy link
Member

Describe the problem

It is often not enough to catch any error that arises in the application code and to send error 500 to the client.

Describe the solution

Better solution would be to allow custom API handler wrappers to be provided

Alternatives

No response

Additional context

See https://github.com/metatech-university/NodeJS-Application/pull/4/files#diff-76374afb32b513f9f1f3ad04638ffcfba3c4cd36aa447236a128c2b6ed6c0936R14

@tshemsedinov
Copy link
Member

tshemsedinov commented Jun 21, 2023

Concerning errors: I think we can use single Error class with following features:

export interface ErrorOptions {
  code?: number | string;
  cause?: Error;
}

export class Error extends global.Error {
  constructor(message: string, options?: number | string | ErrorOptions);
  message: string;
  stack: string;
  code?: number | string;
  cause?: Error;
}

@mprudnik
Copy link

Another option:

class DomainError extends Error {
...
}

class MessengerError extends DomainError {
...

  static invalidPermission() {
    return new MessengerError('Permission denied');
  }
  
  static notFound() {
    return new MessengerError('Not found');
  }
  
  // and so on...
}

This neither requires defining a class per each error (1 proposal) nor requires additional functionality to check if a general purpose error received an allowed message (2 proposal).
This also adds static checks and autocompletion.

@georgolden
Copy link
Member

I suggest to have onError handler for API method like this:

({
  parameters: {
    a: 'number',
    b: 'number',
  },

  method: async ({ a, b }) => {
    const result = a + b;
    return result;
  },

  onError: (error) =>
    'code' in error ? error : { message: error.message, code: 500 },

  returns: 'number',
});

The goal is to have any custom error handlers in case if API method failed.

Inspired from Fastify hooks
https://www.fastify.io/docs/latest/Reference/Hooks/#onerror
https://www.fastify.io/docs/latest/Reference/Lifecycle/

@DemianParkhomenko
Copy link

DemianParkhomenko commented Jun 22, 2023

It is just a draft proposal/example of how we can declare a list of the errors with TypeScript. data: any can be typed better.

export enum ErrorCode {
  notFound = 'NOT_FOUND',
  invalidResponse = 'INVALID_RESPONSE',
  authenticationError = 'AUTHENTICATION_ERROR',
  networkError = 'NETWORK_ERROR',
}

export type ErrorOptions = {
  message: string;
  data?: any;
};

const record: Record<ErrorCode, ErrorOptions> = {
  NOT_FOUND: { message: 'Not found' },
  INVALID_RESPONSE: {
    message: 'Invalid response. Please try again later.',
    data: { additionalFields: 'something' },
  },
  AUTHENTICATION_ERROR: {
    message: 'Authentication failed',
  },
  NETWORK_ERROR: {
    message: 'Network error occurred',
  },
};

export const createError = (code: ErrorCode) => ({ code, ...record[code] });

console.log(createError(ErrorCode.notFound));

@tr3nbolon3
Copy link

tr3nbolon3 commented Jun 22, 2023

// error names
const DOMAIN_ERROR_NAME = {
  NotFound = 'NotFound',
  Forbidden = 'Forbidden',
  SomeEntityIsDeactivated = 'SomeEntityIsDeactivated',
  ...
};

// domain layer
throw new DomainError(DOMAIN_ERROR_NAME.NotFound)

@lundibundi
Copy link
Member

export interface LogFormatError {
  toJSON(): Record<string, unknown>;

  // compatible with https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#tojson_behavior.
}

export interface UserFormatError {
  toUserError(): Record<string, unknown>;

  // allows to provide custom data when error is sent to user
}

both interfaces are optional for all Errors. Logger/API will check for those upon handling.

@tshemsedinov
Copy link
Member

Endpoint code:

({
  parameters: {
    person: { domain: 'Person' },
    address: { domain: 'Address' },
  },

  method: async ({ person, address }) => {
    const addressId = await api.gs.create(address);
    person.address = addressId;
    const personId = await api.gs.create(person);
    return personId;
  },

  returns: { type: 'number' },

  errors: {
    ECONTRACT: 'Invalid arguments',
    ECANTSAVE: 'Person and address can not be saved to database',
    ERESULT: 'Invalid result',
  },
});

We can generate example.d.ts automaticaly

type Code = 'ECONTRACT' | 'ECANTSAVE' |  'ERESULT';
class CustomError {
  constructor(message: string, options: { code: Code });
}

Screenshot from 2023-06-23 04-31-56

@tshemsedinov
Copy link
Member

tshemsedinov commented Jun 23, 2023

I believe, we need complex examples, not just a code snippet, we need branch/fork with a few files to show how it will work and ready to try branch, for example my proposal: metarhia/Example#233
@lundibundi @nechaido @mprudnik @georgolden @DemianParkhomenko @Haliont

@mprudnik
Copy link

mprudnik commented Jun 25, 2023

Added my extended proposal - metarhia/Example#234.
Additional notes regarding error hooks:
I propose to have multiple places, where onError handler can be defined - in method itself (near handler) and error.js file on service level. Then we can trigger the nearest one in case of error:

  • if method defines own onError - use it and ignore service and global error handler
  • else if service defines own onError - use it and ignore global error handler
  • else use global error handler.

Also, I'm not sure if those handlers should be triggered if error is an instance of DomainError since normally that means that it is part of expected behavior and the error can be serialized and sent to the client.

@tshemsedinov tshemsedinov added this to the Version 3.0 milestone Jun 26, 2023
tshemsedinov added a commit that referenced this issue Jun 30, 2023
tshemsedinov added a commit that referenced this issue Jun 30, 2023
tshemsedinov added a commit that referenced this issue Jun 30, 2023
@tshemsedinov
Copy link
Member

Implemented in Version 3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants