yo icon indicating copy to clipboard operation
yo copied to clipboard

Feature: More customizable error handling

Open TarSzator opened this issue 1 year ago • 11 comments

Not sure if this is the right spot for my request.

Starting from the following issue: We use our own error classes. These have an ID which is a number and a code which is an enumeration. When these errors are thrown the error message is written but we also get this error:

TypeError [ERR_INVALID_ARG_TYPE]: The "code" argument must be of type number. Received type string ('InternalError')
    at process.set [as exitCode] (node:internal/bootstrap/node:123:9)
    at process.exit (node:internal/process/per_thread:180:24)
    at onError (/opt/homebrew/lib/node_modules/yo/lib/cli.js:117:11)
    at /opt/homebrew/lib/node_modules/yo/lib/cli.js:168:54
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async init (/opt/homebrew/lib/node_modules/yo/lib/cli.js:168:5)
    at async pre (/opt/homebrew/lib/node_modules/yo/lib/cli.js:83:3)
    at async /opt/homebrew/lib/node_modules/yo/lib/cli.js:193:3

Goal: I would like to influence the error handling from yeoman. So I would like to provide a function to yeoman that is triggered with the error and what ever I return is printed to the console.

Advantage: With this I could prevent the check if code is a number. And I could also improve the output to be more then just the message property of my error

Question: Does already exist a way to do this? I did not find it in the documentation.

Note: I am aware of the error event but this does not help me with the code error issue.

TarSzator avatar Sep 05 '24 16:09 TarSzator

This issue is stale because it has been open with no activity. Remove stale label or comment or this will be closed

github-actions[bot] avatar Oct 06 '24 00:10 github-actions[bot]

This looks an yo issue. Looks like yo is forward the error code to process.exit().

mshima avatar Oct 06 '24 00:10 mshima

This issue is stale because it has been open with no activity. Remove stale label or comment or this will be closed

github-actions[bot] avatar Nov 07 '24 00:11 github-actions[bot]

Is there some action planed by the maintainers? Would also like to get the stale tag removed 😃

TarSzator avatar Nov 07 '24 07:11 TarSzator

This issue is stale because it has been open with no activity. Remove stale label or comment or this will be closed

github-actions[bot] avatar Dec 08 '24 00:12 github-actions[bot]

I will probably move to yo repository, we may have some news about yo maintenance soon.

mshima avatar Dec 08 '24 00:12 mshima

This issue is stale because it has been open with no activity. Remove stale label or comment or this will be closed

github-actions[bot] avatar Jan 09 '25 00:01 github-actions[bot]

👋 We do have news about yo maintenance! #838.

@TarSzator I'd like to learn more. Could you please post a link to your project and say more about how you've set up your error classes? How do they get thrown now, how you'd like them to impact Yeoman, and any other context you think is relevant?

JoshuaKGoldberg avatar Jan 23 '25 22:01 JoshuaKGoldberg

@JoshuaKGoldberg Awesome to hear from you 🥳 😃

Since these a company projects I can not share a link, but I will provide more context and information if not tomorrow then over the weekend. I will definitely clear tomorrow with my manager what I can share here 😃

TarSzator avatar Jan 23 '25 23:01 TarSzator

@JoshuaKGoldberg Sorry for the late answer, life got in the way 😄

Regarding our Error classes:

I will share our BaseError "class" here to give you an idea of it

import { isNonEmptyString, isNumber, isString } from '@softgames/type-guards';
import { ErrorCode } from '../enums/index.js';
import { isErrorCode } from '../guards/index.js';
import {
  ErrorObject,
  ErrorResponseBody,
  PlainError,
  AdditionalInformation,
} from '../types/index.js';

export class BaseError extends Error {
  readonly timestamp: number;

  readonly code: ErrorCode;

  readonly id: number;

  additionalInformation?: AdditionalInformation;

  readonly parentError?: unknown;

  constructor(
    code: ErrorCode,
    id: number,
    message: string,
    additionalInformation?: AdditionalInformation,
    parentError?: PlainError,
  ) {
    super(message);
    this.timestamp = Date.now();
    this.id = isNumber(id) ? id : 1647962470;
    this.code = isErrorCode(code) ? code : ErrorCode.UnknownError;
    this.additionalInformation = additionalInformation;
    this.parentError = parentError;
  }

  toErrorResponse(): ErrorResponseBody {
    const obj = objectifyError(this);
    const { message, code, id, additionalInformation, timestamp } = obj;
    return {
      message,
      code: code || ErrorCode.UnknownError,
      id: id || 1663755346,
      ...(additionalInformation?.public
        ? { additionalInformation: additionalInformation.public }
        : {}),
      timestamp: timestamp || Date.now(),
    };
  }

  toObject(secure = false): ErrorObject {
    const obj = objectifyError(this);
    if (!secure) {
      return obj;
    }
    const { stack, parentError, ...clean } = obj;
    return clean;
  }

  toString(): string {
    return JSON.stringify(this.toObject(), null, '  ');
  }

  valueOf(): ErrorObject {
    return this.toObject();
  }

  get [Symbol.toStringTag](): string {
    return this.toString();
  }
}

function stackToArray(stack: string): string[] | undefined {
  if (!stack) {
    return undefined;
  }
  return String(stack)
    .split('\n')
    .map((s) => s.trim())
    .slice(1);
}

function prepareMessage(error: unknown): string {
  const { message } = (error || {}) as Error;
  if (isNonEmptyString(message)) return message;
  if (isNonEmptyString(error)) return error;
  return castSerializable(error);
}

const SERIALIZE_ERROR = 'BaseError handling issue: Error was not serializable';

function castSerializable(error: unknown): string {
  try {
    const stringifiedError = JSON.stringify(error);
    if (isString(stringifiedError)) {
      return stringifiedError;
    }
    return SERIALIZE_ERROR;
  } catch (err) {
    return SERIALIZE_ERROR;
  }
}

function objectifyError(error: unknown, depth = 1): ErrorObject {
  const { code, id, stack, additionalInformation, parentError, timestamp } =
    error as BaseError;
  return {
    message: prepareMessage(error),
    ...(code ? { code } : {}),
    ...(id ? { id } : {}),
    ...(timestamp ? { timestamp } : {}),
    ...(stack ? { stack: stackToArray(stack) } : {}),
    ...(additionalInformation ? { additionalInformation } : {}),
    ...(parentError
      ? {
          parentError:
            depth <= 10
              ? objectifyError(parentError, depth + 1)
              : {
                  message:
                    'Truncated further parent errors due to reaching max depth',
                },
        }
      : {}),
  };
}

The general idea is that one has to provide a unique ID the moment when one creates a new error. Usually an EPOCH.

try {
  // to stuff
} catch (error: unknown) {
  throw new InternalError(1738445805, 'Failed to do stuff', { params }, error);
}

This way over all our project when we get this ID we can uniquely identify the source of the error over project borders. As you notice the error code is backed into the specific error classes that are extending the BaseError class.

This should explain our main issue in regards to the code property.

Regarding the general error handling inside of yeoman:

We have a lot of generators that are classes extending Generator

import Generator from 'yeoman-generator';

and as intended we are then providing a lot of methods that do all the magic that we want our generator to do.

Now naturally sometimes our methods throw an error and a Promise returned by our methods is rejected. Sadly we can not manage how this error is handled.

I would love to provide a function to yeoman to manipulate any error handling.

On idea could be a method of the Generator class that can be overwritten by any class that extends it OR a function that can be provided on construction of the Generator instance.

My idea would be a function with this signature

type ErrorHandler = (methodName: string, error: unknown) => string

This function should then always be executed when an error is thrown by a method or a returned Promise is rejected and its output is printed to the console. Multiline and colors should be supported.

Extended version: maybe even with exit code, maybe even with Promise support

type ErrorHandlerResponse = string | { exitCode: number; message: string }

type ErrorHandler = (methodName: string, error: unknown) => ErrorHandlerResponse | Promise<ErrorHandlerResponse>

Did I made my idea and point clear? If you have any question or want to do some idea improvement ping pong I am 100 % open for that

TarSzator avatar Feb 01 '25 21:02 TarSzator

Thanks, this is really helpful context! I think I understand the technical underpinning of what you're trying to do. It sounds like there are two issues in play:

  • Bug report: throwing an error with a .code string crashes Yeoman unhappily
  • Feature request: more customizeable/robust error handling system

Answering a few points in order...

These have an ID which is a number and a code which is an enumeration.

We can see from the call stack that this comes from:

https://github.com/yeoman/yo/blob/87498471eaf8b1b85d80e54416f9961f9cf2f6a4/lib/cli.js#L117

https://nodejs.org/api/errors.html#err_invalid_arg_type:

An argument of the wrong type was passed to a Node.js API.

So, it looks like Yeoman requires error.code to be type number | null | undefined. The code property is meant to be used for what's forwarded to process.exit(): i.e. what was said in https://github.com/yeoman/yo/issues/837#issuecomment-2580043747. Putting a string there is invalid.

Aside: I don't recall this being documented anywhere. Maybe that's a good docs issue to file on this repo or https://github.com/yeoman/yeoman.io...? Something around the docs for building generators.

This way over all our project when we get this ID we can uniquely identify the source of the error over project borders.

Checking: have you looked at Error.cause? I'm wondering if part of what you're doing with InternalError is duplicated by that property - or if that's already what you're using?

I would love to provide a function to yeoman to manipulate any error handling. ... This way over all our project when we get this ID we can uniquely identify the source of the error over project borders.

Although I think I understand the technical bits, what's missing here is the why. Yeoman will log the .message property (or .stack if debugging is enabled). You can use that today to print whatever string properties you want.

So I think what I'm waiting for here is: what is it that you can't log today that a more robust error handling would enable?

JoshuaKGoldberg avatar Feb 06 '25 18:02 JoshuaKGoldberg

Closing out as non-actionable. If anybody else has a reproduction / more information, please do post back. Cheers! 🎩

JoshuaKGoldberg avatar Nov 05 '25 14:11 JoshuaKGoldberg