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

VError.info doesn't return custom properties on non-VError cause children #79

Open
jjm340 opened this issue Aug 20, 2020 · 5 comments
Open

Comments

@jjm340
Copy link

jjm340 commented Aug 20, 2020

I have an error class that adds custom properties to itself:

export class ValidationError extends Error {
  public responseCode = ResponseCode.ValidationError;
  constructor(message: string, public data: any) {
    super(message);
  }
}

I have a logger method thats supposed to grab all the keys and scrub them from the error chain:

exception: (message: string, ex: Error, data?: Record<string, unknown>) => {
    const errorToLog = new VError(
      {
        cause: ex,
        info: data,
      },
      message,
    );

    const info = VError.info(errorToLog);
    console.log('info: ', info);

    logError(message, errorToLog);
  }

But when I pass an instance of the first error as the cause to VError, the VError.info method doesn't enumerate the keys for the ValidationError. Is this by design?

@davepacheco
Copy link
Contributor

If I'm understanding right, that's the way it's intended. This bit in the README explains why we did it this way:

In order for all this to work, programmers need to know that it's generally safe to wrap lower-level Errors with higher-level ones. If you have existing code that handles Errors produced by a library, you should be able to wrap those Errors with a VError to add information without breaking the error handling code. There are two obvious ways that this could break such consumers...
The error's properties might change. People often hang additional properties off of Error objects. If we wrap an existing Error in a new Error, those properties would be lost unless we copied them. But there are a variety of both standard and non-standard Error properties that should not be copied in this way: most obviously name, message, and stack, but also fileName, lineNumber, and a few others. Plus, it's useful for some Error subclasses to have their own private properties -- and there'd be no way to know whether these should be copied. For these reasons, VError first-classes these information properties. You have to provide them in the constructor, you can only fetch them with the info() function, and VError takes care of making sure properties from causes wind up in the info() output.

Sorry if I'm misunderstanding!

@jjm340
Copy link
Author

jjm340 commented Aug 20, 2020

For these reasons, VError first-classes these information properties. You have to provide them in the constructor, you can only fetch them with the info() function, and VError takes care of making sure properties from causes wind up in the info() output.

This part of the documentation makes it sound like it should surface these custom properties.

@jjm340
Copy link
Author

jjm340 commented Aug 20, 2020

Also what about other libraries that I have no control over?

@davepacheco
Copy link
Contributor

When it says "properties from causes", it's referring to the same first-class informational properties that were provided in the VError constructor on those causes. Right now, you'd have to wrap errors from other libraries if you wanted to preserve this information. It's possible there's a better approach here? But the rest of the text explains our constraints on the problem and why it seemed fraught to just copy all the properties that we find.

@jjm340
Copy link
Author

jjm340 commented Aug 21, 2020

Okay, so I wrap other errors in VError to get those moved up. Got it. Makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants