Unify Error Throwing Methods

[chacha912]

Description:

yorkie-js-sdk uses three different methods for throwing errors. Let's pick one way to throw errors and apply it consistently across yorkie-js-sdk. If there's a better approach, let's talk about it and see if we should switch.

1. throw new Error:

Generates a general error using the throw statement. This is a straightforward and common method.

throw new Error('This is a general error.');

2. throw new YorkieError:

Creates and throws a custom error, allowing additional information to be added to the error. This approach helps identify errors originating from Yorkie, provides a way to categorize errors, but requires handling of error codes for each case.

// src/util/error.ts
export class YorkieError extends Error {
  name = 'YorkieError';
  stack?: string;

  constructor(readonly code: Code, readonly message: string) {
    super(message);
    this.toString = (): string =>
      `${this.name}: [code=${this.code}]: ${this.message}`;
  }
}

throw new YorkieError(Code.Unsupported, `unsupported type: ${valueType}`);

3. logger.fatal():

Records errors using the logger with the fatal level. Different processing can be applied based on log levels.

// src/util/logger.ts
export const logger = {
  fatal: (message: string, ...messages: Array<unknown>): void => {
    if (typeof console !== 'undefined') {
      if (typeof console.error !== 'undefined') {
        console.error('YORKIE F:', ...messages);
      } else {
        console.log('YORKIE F:', ...messages);
      }
    }

    throw new Error(`YORKIE F: ${message}`);
  },
  // Other log levels like debug, info, warn, error, etc.
};

logger.fatal('This is a fatal error.');

However, note that an error is being thrown within logger.fatal. This can lead to TypeScript errors when using the method, as showed in the example below:

// operation.ts
public getExecutedAt(): TimeTicket {
  if (!this.executedAt) {
    logger.fatal(`executedAt has not been set yet`);
  }
  return this.executedAt; 
// ^^^^^^  Type 'TimeTicket | undefined' is not assignable to type 'TimeTicket'.
}

Why:

Standardize the error-throwing approach for consistency and clarity throughout the yorkie-js-sdk codebase.

[gwbaik9717]

Could I give it a try?

[krapie]

@gwbaik9717 Sure! Happy to see you doing error related issues again.

[gwbaik9717]

I'm considering solving this issue as follows. Please review and let me know if it is alright.

I believe the logger should focus solely on logging and not be responsible for throwing errors.

I suggest logging messages with the logger and throwing errors separately using YorkieError.

After the change, there will be only two scenarios in logging and throwing errors:

1. Case when only throwing Error

throw new YorkieError(Code.ErrInvalidArgument, ERROR_MESSAGE);

2. Case when logging and throwing Error

// src/util/logger.ts
export const logger = {
  fatal: (message: string, ...messages: Array<unknown>): void => {
    if (typeof console !== 'undefined') {
      if (typeof console.error !== 'undefined') {
        console.error('YORKIE F:', ...messages);
      } else {
        console.log('YORKIE F:', ...messages);
      }
    }
     // throw error removed
  },
  // Other log levels like debug, info, warn, error, etc.
};

const ERROR_MESSASGE = "This is a fatal error."

logger.fatal(ERROR_MESSAGE);
throw new YorkieError(Code.ErrInvalidArgument, ERROR_MESSAGE);

In this way, I believe we can solve the type problem of the logger function, and users will be explicitly aware when an error is being thrown.

@chacha912

[chacha912]

@gwbaik9717 Thank you for sharing your approach. As you suggested, it would be good to standardize the error handling by throwing custom YorkieError, and use the logger for all logging purposes.

cc. @hackerwins