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

Feature: More customizable error handling #837

Open
TarSzator opened this issue Sep 5, 2024 · 11 comments
Open

Feature: More customizable error handling #837

TarSzator opened this issue Sep 5, 2024 · 11 comments

Comments

@TarSzator
Copy link

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.

This comment has been minimized.

@mshima
Copy link
Member

mshima commented Oct 6, 2024

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

This comment has been minimized.

@TarSzator
Copy link
Author

TarSzator commented Nov 7, 2024

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

This comment has been minimized.

@mshima
Copy link
Member

mshima commented Dec 8, 2024

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

This comment has been minimized.

@mshima mshima transferred this issue from yeoman/generator Jan 9, 2025
@JoshuaKGoldberg
Copy link

👋 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?

@TarSzator
Copy link
Author

@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
Copy link
Author

@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

@JoshuaKGoldberg JoshuaKGoldberg changed the title Error handling Feature: More customizable error handling Feb 6, 2025
@JoshuaKGoldberg
Copy link

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:

yo/lib/cli.js

Line 117 in 8749847

process.exit(error.code || 1);

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 #837 (comment). 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?

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

3 participants