Skip to content

Throwing a custom error objects sets culprit / last stack frame to error message #1209

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

Closed
vicrep opened this issue Jan 30, 2018 · 5 comments
Closed

Comments

@vicrep
Copy link

vicrep commented Jan 30, 2018

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

A project I work on (in typescript) defines custom ES6 errors as such:

export class CustomError extends Error {
  name = 'CustomError';
  constructor(...args) {
    super(...args);
    Error.captureStackTrace(this, Error);
  }
}

Now, throwing the custom error (throw new CustomError('my error message')) and inspecting the data sent (through dataCallback, we noticed that both data.culprit and the last frame in data.exception.values[0].stacktrace.frames both point to the filename CustomError my error message, with the file which throws the error being the second last entry in the stacktrace.

In comparison, throwing a standard error yields a data.culprit and last stacktrace frame both pointing to whichever file threw the error.

What is the expected behavior?

Custom errors should be handled the same way as standard errors. We believe the impact of this is that it prevents proper grouping of those custom errors, and we went around this by popping the last stacktrace frame and overriding the culprit in the dataCallback when this scenario is detected.

Using Raven.JS 3.22.1, installed through NPM.

@kamilogorek
Copy link
Contributor

Hey @vicrep, this should be covered by TraceKit that we use. However, I see that you're not creating CustomError correctly (you are missing prototype chain assignments). Could you give this code a shot and let me know if you still have the same issue? (also, please provide stacktrace and culprit screenshots if possible).

export class SentryError extends Error {
  public name: string;

  constructor(public message: string) {
    super(message);
    this.name = new.target.prototype.constructor.name;
    Object.setPrototypeOf(this, new.target.prototype);
  }
}

@vicrep
Copy link
Author

vicrep commented Feb 9, 2018

@kamilogorek thanks for getting back to me.

So I took the time to try your suggested fix, and ended up discovering what was causing the actual issue.

Prototype chain assignments were actually being handled by the typescript compiler, so doing Object.setPrototypeOf(this, new.target.prototype); wasn't necessary.

In my issue request, I wrote CustomError as a generic example, but in fact, the custom error I was actually using in my project was called HttpError. Further investigation led me to discover that having the class name begin with Http was actual the culprit, and causes sentry's stack trace collection (i.e. tracekit?) to fail.

Here's an example, where I have two custom errors, declared in the same file, which share the exact same implementation -- one is named HttpError, the other HttError (no p).

Throwing HttpError somewhere in my project, this is the culprit and last stack frames I get from sentry's dataCallback:
screen shot 2018-02-09 at 10 25 46 am
screen shot 2018-02-09 at 10 26 05 am

Now, throwing the error HttError from the exact same place, here's what I get:
screen shot 2018-02-09 at 10 27 32 am
screen shot 2018-02-09 at 10 27 47 am

So it seems something is happening in the pipeline when errors' class names begin with Http

@kamilogorek
Copy link
Contributor

Well, that's definitely unexpected behavior haha. Thanks for the investigation.
Neither Raven.js, nor Tracekit is doing anything that could cause an issue like this.

I wonder if it's TypeScript compiler then or some part of your Webpack config.

Are you able to provide some minimal reprop case that I could use to fiddle with it? It'd be nice to find the main cause for a future reference.

@vicrep
Copy link
Author

vicrep commented Feb 15, 2018

Hey @kamilogorek I checked with my TypeScript compiler, and the generated classes were the exact same for Htt and Http (it's a big project, so sharing a copy of the config is non-trivial). I however decided to give it a shot using vanilla JS, and can confirm that it's an issue with TraceKit (or the version of TraceKit being used internally by Sentry).

Here's a repro for you. Note that what actually triggers the wrong stack trace is when the name property of the error instance starts with Http -- not setting this.name = 'HttpError' (but keeping the class name) makes the issue go away.

https://plnkr.co/edit/tY7Wf39GcnodxFq6ylOs?p=preview

@kamilogorek
Copy link
Contributor

I was indeed nasty TraceKit regexp bug.
#1237 will fix it once merged, so let me close this one.

Thanks for investigating it @vicrep. Cheers!

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