line-bot-sdk-nodejs icon indicating copy to clipboard operation
line-bot-sdk-nodejs copied to clipboard

Refactoring Exception

Open pengooseDev opened this issue 1 year ago • 0 comments

PR Context

This PR is based on Issue #699 to Refactoring exception(Error class)

Need Feedbacks

1. unused Field (originalError)

export class RequestError extends Error {
  public code: string;

  private originalError: Error; // ⛳️ Not used private field

  constructor(message: Message, { code, originalError }: AxiosErrorDetails) {
    super(message);
    this.name = this.constructor.name;

    Object.assign(this, { code, originalError });
  }
}

RequestError's originalError is a private field, but it is not being used. Looking at the commit history, it seems that there are cases where the originalError field that was private is converted to public. I need feedback whether to keep or remove the field. (It doesn't matter if you keep the current code, but if you think you don't need it, we will remove it.)

2. CodeConvention

In the process of improving the class, there was no information on the code convention, so we applied Airbnb's code convention (such as applying the extension to the class member variable) lightly. However, if you believe that excessive modification rather harms readability, I will remove all modifications. Please feedback on this as well!

export class HTTPFetchError extends Error {
  public status: number;

  public statusText: string;

  public headers: Headers;

  public body: string;

  constructor(
    message: Message,
    { status, statusText, headers, body }: FetchErrorDetails,
  ) {
    super(message);
    this.name = this.constructor.name;

    Object.assign(this, { status, statusText, headers, body });
  }
}

3. Interface

I tried to declare all the attributes of each error inside one interface, but I removed the interface. If you think it is excessive, I will manage it on one interface. :)

type Message = string;

interface Status {
  status: number;
  statusText: string;
}

interface ErrorDetails {
  signature?: string;
  raw?: any;
}

interface FetchErrorDetails extends Status {
  headers: Headers;
  body: string;
}

// Deprecated
interface AxiosErrorDetails extends Partial<Status> {
  originalError: Error;
  code?: string;
}

After reviewing the code, I would appreciate it if you could discuss the direction of the code and request further changes.

pengooseDev avatar Mar 06 '24 13:03 pengooseDev