-
-
Notifications
You must be signed in to change notification settings - Fork 406
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
Comments
This comment has been minimized.
This comment has been minimized.
This looks an yo issue. |
This comment has been minimized.
This comment has been minimized.
Is there some action planed by the maintainers? |
This comment has been minimized.
This comment has been minimized.
I will probably move to yo repository, we may have some news about yo maintenance soon. |
This comment has been minimized.
This comment has been minimized.
👋 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 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 😃 |
@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. This should explain our main issue in regards to the 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. 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 |
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:
Answering a few points in order...
We can see from the call stack that this comes from: Line 117 in 8749847
https://nodejs.org/api/errors.html#err_invalid_arg_type:
So, it looks like Yeoman requires 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.
Checking: have you looked at
Although I think I understand the technical bits, what's missing here is the why. Yeoman will log the 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? |
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:
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.
The text was updated successfully, but these errors were encountered: