getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.99k stars 1.57k forks source link

Non-Error exception captured with keys: error, headers, message, name, ok #2292

Closed EternalPirate closed 4 years ago

EternalPirate commented 5 years ago

Package + Version

Version:

5.7.1 (@sentry/browser)
8.2.11 (@angular/core)

Description

I have initial setup for Angular app with global ErrorInterceptor here how I send it

    const eventId = Sentry.captureException(error.originalError || error);
    Sentry.showReportDialog({ eventId });

and I get this (Non-Error exception captured with keys: error, headers, message, name, ok) error again and again, and can't understand what is wrong from the description and how to reproduce it.

kamilogorek commented 5 years ago

This means that the object you provide is not an instance of Error which holds the stacktrace in Angular app.

You should (as the message points to) use Sentry.captureException(error.error) or Sentry.captureException(error.message) depending on your needs.

EternalPirate commented 5 years ago

@kamilogorek oh ok) tnx

EternalPirate commented 5 years ago

@kamilogorek still getting this error, even with this

    const exception = error.error || error.message || error.originalError || error;
    const eventId = Sentry.captureException(exception);
Zeswen commented 4 years ago

+1, still getting this error no matter how I put it.

sentry.error-handler.ts

export class SentryErrorHandler extends GeneralErrorHandler {
...
handleError(error) {
...
const exception = error.originalError || error.error || error
Sentry.captureException(exception)
}
}

package.json

{
...
"@angular/core": "^8.2.11",
"@sentry/browser": "^5.7.1",
...
}
EternalPirate commented 4 years ago

I decided to do this

Sentry.init({
            dsn: environment.sentryUrl,
            beforeSend(event, hint) {
                /* tslint:disable:no-string-literal only-arrow-functions */
                const isNonErrorException =
                    event.exception.values[0].value.startsWith('Non-Error exception captured') ||
                    hint.originalException['message'].startsWith('Non-Error exception captured');
                /* tslint:enable:no-string-literal only-arrow-functions */

                if (isNonErrorException) {
                    // We want to ignore those kind of errors
                    return null;
                }
                return event;
            }
        });
Zeswen commented 4 years ago

Thank you for the workaround @gchronos! I hope there will be a potential fix anytime soon.

louieogrady commented 4 years ago

Just chiming in to say I am also getting these errors. I'm using Angular 8 and after reading quite a few issues on this, I realise that it might be a case of me not handling the error properly in my error handling component. I've tried the workarounds suggested by others defining the exception before being passed to captureException but this hasn't reduced the errors. It'd be great if anyone could give some further input on this or I'll just have to use GChronos's (Thanks!) solution.

franbueno commented 4 years ago

+1

stephane-dereppe commented 4 years ago

+1

kamilogorek commented 4 years ago

Can someone provide repro that I could use to debug this? I tried to use base angular-cli app and I cannot reproduce this behavior. Thanks!

xemil commented 4 years ago

@kamilogorek I believe you can init a new Angular 8 project, and make a http request towards a endpoint returning 500 and not catch the error in the http service so it propagates to Sentry.

jonathan-payiq commented 4 years ago

I can chime in with some more info, this is our ErrorHandler:

@Injectable()
export class SentryErrorHandler implements ErrorHandler {
  constructor() { }
  handleError(error) {
    Sentry.captureException(error.originalError || error.error || error);
  }
}

In our Sentry tracking we always get duplicate events for these ones:

(Other events are reported okay as one item in the list)

We have collected a couple hundreds of these "Non.Error"-events, and interestingly they all have the following in common:

And the errors are mostly the following:

{
  error: [Object], 
  headers: [Object], 
  message: Http failure during parsing for https://foo.bar/baz, 
  name: HttpErrorResponse, 
  ok: False, 
  status: 200, 
  statusText: OK, 
  url: https://foo.bar/baz
}
hdayacr commented 4 years ago

Ok I rendered the SentryErrorHandler injectable and implements ErrorHandler instead of extends and there are no more problems like that for me. So I changed from

export class SentryErrorHandler extends ErrorHandler {

 constructor() {
     super();
 }

 handleError(err: any): void {
     if (environment.production === true || environment.preprod === true) {
         Sentry.captureMessage(err.originalError || err);
     }
     throw err;
 }
}

to

@Injectable()
export class SentryErrorHandler implements ErrorHandler {

 constructor() { }

 handleError(err: any): void {
     if (environment.production === true || environment.preprod === true) {
         Sentry.captureException(err.originalError || err);
     }
     throw err;
 }
}

I had the first config for 2 years without problems but since the upgrade to Angular 8 I have several sentry exceptions.

schumannd commented 4 years ago

@jonathan-payiq have you been able to find out what causes this? Or do you just ignore all those Non-Error exceptions now?

jonathan-payiq commented 4 years ago

We are currently ignoring them. We figured out from the breadcrumbs that these happen when the webview/browser is in the background (not visible), so the fix for us is to prevent the fetch from happening while the page is not visible. Apparently Samsung browsers handle the network refresh badly when in background.

mihaic195 commented 4 years ago

@jonathan-payiq how exactly are you ignoring them? Thanks in advance.

szechyjs commented 4 years ago

We are currently using the following to ignore it.

Sentry.init({
  ignoreErrors: [
    'Non-Error exception captured'
  ]
});
rhcarvalho commented 4 years ago

@kamilogorek were you able to reproduce the issue?

kamilogorek commented 4 years ago

Closing the issue, as it seems like the original issue has been partially resolved or there is a working solution. I'd prefer someone to create a new issue with a fresh description if it's still an issue. Please do not hesitate to ping me if it is still relevant, and I will happily reopen and work on it. Cheers!

Rush commented 4 years ago

It has not been resolved. I just tried to integrate Sentry with Angular 8 and found this issue.

Rush commented 4 years ago

I don't get it how can Sentry claim out of the box setup, it's completely false.

kamilogorek commented 4 years ago

I don't get it how can Sentry claim out of the box setup, it's completely false.

@Rush provide a reproducible case, where you explain and show what's wrong, and then we can verify your claim.

Sija commented 4 years ago

@kamilogorek Truth to be told, that ain't working with Angular since over 6 months, there's enough of reports here to confirm it, thus IMO there's no need for further repro cases, you can find it out by yrself pretty easily. I know myself it doesn't, since we have the same issue in our project using Angular v8, and now v9 (no sourcemaps).

Rush commented 4 years ago

Exactly, and besides I'm paying for Sentry to do the work for me. It's not an open source project where I do expect to get my hands dirty.

I like Sentry but I could look for alternatives...

kamilogorek commented 4 years ago

I used an https://github.com/gothinkster/angular-realworld-example-app app as an example, just so that everyone can also do the same reproduction.

Barebone cloned app, followed Sentry's Angular docs and uploaded the sourcemaps. Here are the results.


Example 1: Broken logic

What Angular gives you in an error handler + what event we capture:

Screenshot 2020-04-16 at 12 01 13

How it looks in the UI:

Screenshot 2020-04-16 at 12 07 02

Note: Everything is in place, you can easily point to the correct line of code that was broken, error message and error type is correct, because Angular gives you a whole Error object to work with.


Example 2: Broken service call

What Angular gives you in an error handler + what event we capture:

Screenshot 2020-04-16 at 12 00 49

How it looks in the UI:

Screenshot 2020-04-16 at 12 07 12 Screenshot 2020-04-16 at 12 08 03

Note: There's no way to tell what type of error has been thrown, because Angular doesn't give you this information. However, you can take a look at serialized data to tell, that there was "404 Not Found" error, which most likely came from invalid XHR call. Then, you can investigate the breadcrumbs flow to see that in fact there was an XHR call to the given url (which in this case contained a typo), preceeded by a click on an exact DOM element, which triggered it. With this information, it should be enough in 99% cases to fix the issue.


We can only work with what we've been given by the framework. Otherwise, we'd have to monkey-patch the framework itself.

szechyjs commented 4 years ago

We can only work with what we've been given by the framework. Otherwise, we'd have to monkey-patch the framework itself.

Is this your way of saying "not our problem"? Before switching to sentry (from some competitors) these XHR errors were captured just fine and they used the same install method with an angular error handler.

Rush commented 4 years ago

@kamilogorek as a vendor you could also open a ticket with the framework. I'm sure it would get some attention.

kamilogorek commented 4 years ago

Before switching to sentry (from some competitors) these XHR errors were captured just fine and they used the same install method with an angular error handler.

@szechyjs then it'd be great if you could share the solution instead, as it's open-source SDK and we can iterate over improvements if we know what we are aiming for.

szechyjs commented 4 years ago

... you could also open a ticket with the framework. I'm sure it would get some attention.

I just opened a ticket with sentry support and referenced this issue.

tsuz commented 4 years ago

A passerby here. For my case, I want to report handled rejections in react native and I ended up seeing the same error as the OP. Since the object you provide is not an instance of Error, I got around using the below.

export interface IError extends Error {
  code?: ErrorCode
  msg?: string
}

export const appError = (code: ErrorCode, msg: string = ''): IError => {
  const errmsg = `code: ${code}, msg: ${msg}`
  const err: IError = new Error(errmsg)
  err.code = code
  err.msg = msg
  err.name = 'appError'
  return err
}

Then I call

export const logError = (err: Error) => {
  sentry.captureException(err)
}

and I see

スクリーンショット 0032-05-24 1 58 38

Hope this helps!

mihaic195 commented 4 years ago

In my case the Breadcrumbs section from Sentry helped me a lot to pinpoint the cause. It mostly happened when there were some 404 on some external APIs.

kamilogorek commented 4 years ago

Docs update and necessary JS SDK change are coming:

https://github.com/getsentry/sentry-docs/pull/1695/ https://github.com/getsentry/sentry-javascript/pull/2601

It should be released this week! :)

johnhamm commented 4 years ago

Easy fix for this (as this only happens with HTTP intercepted errors):

handleError(error: Error | HttpErrorResponse | any) {
      // ... 
      if (error.constructor.name === "HttpErrorResponse") {
                error.error.Message + " (" + error.message + ")";
                error = error.error;
      }
      // ...
      Sentry.captureMessage(err.originalError || err.error || error);

     throw err;
 }
kamilogorek commented 4 years ago

New docs and 5.16.0 just has been released - https://docs.sentry.io/platforms/javascript/angular/ I tried to make it as explicit and detailed as possible, and that's why it "may look" like a lot of code.

sblawrie commented 4 years ago

@kamilogorek I'm having this issue on a Vue app, too. Is there a fix over there?

cincauhangus commented 4 years ago

Same here. the exception is thrown in the vuejs sentry integration error handler. https://sentry.io/share/issue/eaf13a2455e04150aaaab595d0d7bafe/

kamilogorek commented 4 years ago

@sblawrie what issue exactly do you have? Some more details would be helpful please. @cincauhangus it looks like somewhere in your code you are throwing an error with the promise instance itself (note abort, always, catch, done keys).

Something like:

const promise = new Promise((resolve, reject) => (...));
// somehow somewhere in the code
function foo () {
  // ...
  throw promise
}

So the errorHandler of Vue is getting a whole promise object, where it expects to be an Error instance.

cincauhangus commented 4 years ago

@kamilogorek thanks for the clarification. I've made some changes and will monitor if it's still an issue.

lethargicgeek commented 4 years ago

Using Angular 8 and had the same issue Non-Error exception. Modified the example 'extractError' code as follows:

private static extractError(error: any) {
    // Try to unwrap zone.js error.
    // https://github.com/angular/angular/blob/master/packages/core/src/util/errors.ts
    if (error && error.ngOriginalError) {
      error = error.ngOriginalError;
    }
    // We can handle messages and Error objects directly.
    if (typeof error === 'string' || error instanceof Error) {
      return error;
    }
    // If it's http module error, extract as much information from it as we can.
    if (error instanceof HttpErrorResponse) {
      // The `error` property of http exception can be either an `Error` object, which we can use directly...
      if (error.error instanceof Error) {
        return error.error;
      }
      // ... or an`ErrorEvent`, which can provide us with the message but no stack...
      if (error.error instanceof ErrorEvent) {
        return error.error.message;
      }
      // ...or the request body itself, which we can use as a message instead.
      if (typeof error.error === 'string') {
        return `Server returned code ${error.status} with body "${error.error}"`;
      }
      // If we don't have any detailed information, fallback to the request message itself.
      return error.message;
    }

    // ***** CUSTOM *****
    // The above code doesn't always work since 'instanceof' relies on the object being created with the 'new' keyword
    if (error.error && error.error.message) {
      return error.error.message;
    }
    if (error.message) {
      return error.message;
    }
    // ***** END CUSTOM *****

    // Skip if there's no error, and let user decide what to do with it.
    return null;
  }

Modified my Sentry.init beforeSend based on @untilinvite's beforeSend example above. This change is to handle a second log message from https://github.com/getsentry/sentry-javascript/issues/2169 also resolving as a Non-Error.

 Sentry.init({
        dsn: AppConfig.envSettings.sentryDSN,
        maxBreadcrumbs: 50,
        environment: this.getEnvName(),
        integrations: [new Sentry.Integrations.Breadcrumbs({ console: false })],
        beforeSend(event, hint) {
          // Note: issue with double entries during http exceptions: https://github.com/getsentry/sentry-javascript/issues/2169
          // Note: issue with a second entry not being set correctly (as a non-error): https://github.com/getsentry/sentry-javascript/issues/2292#issuecomment-554932519
          const isNonErrorException = event.exception.values[0].value.startsWith('Non-Error exception captured');
          if (isNonErrorException) {
            if (!event.extra.__serialized__) {
              return null;
            }
            let realErrMsg = event.extra.__serialized__.error ? event.extra.__serialized__.error.message : null;
            realErrMsg = realErrMsg || event.extra.__serialized__.message;
            // this is a useless error message that masks the actual error.  Lets try to set it properly
            event.exception.values[0].value = realErrMsg;
            event.message = realErrMsg;
          }
          return event;
        }
      });
szechyjs commented 4 years ago

We have made the changes that were added to the docs with the 5.16.0 release and we are still getting Non-Error exception captured with keys: error, headers, message, name, ok errors. I'm hoping the @sentry/angular package will fix these issues?

Angular: v9.1 Sentry: v5.20

kamilogorek commented 4 years ago

@szechyjs did you also read the second part about http interceptors? This is usually the main issue casing the error to not be detected correctly - https://docs.sentry.io/platforms/javascript/angular/

szechyjs commented 4 years ago

@szechyjs did you also read the second part about http interceptors? This is usually the main issue casing the error to not be detected correctly - https://docs.sentry.io/platforms/javascript/angular/

We aren't using interceptors.

jakkn commented 4 years ago

Hi, I've spent some time on this issue and found that error.error.message on errors of type HttpErrorResponse does not necessarily contain any information. If we look at the constructor we'll see that Angular sets the message prop on the root object and not on ErrorEvent.

https://github.com/angular/angular/blob/cb3db0d31b91bea14c7f567fe7e7220b9592c11b/packages/common/http/src/response.ts#L345

By changing this line https://github.com/getsentry/sentry-javascript/blob/8eb72865dcd62ff719441105c7eda28007a07e9d/packages/angular/src/errorhandler.ts#L112 to

if (error.error instanceof ErrorEvent && error.error.message)

the error handler proceeded to return error.message; and threw the expected error.

kamilogorek commented 4 years ago

Great catch @jakkn, thanks! Updated handler in the PR https://github.com/getsentry/sentry-javascript/pull/2903

sam0x17 commented 3 years ago

I'm having this issue on an Express app I just inherited. No idea how this could happen, but it seems to be happening for most errors the app throws, and I'm using the standard Express setup from the docs.

e.g. https://sentry.io/share/issue/bfd4f674c10b4b4a8b6a291dde8e2a66/

tswaters commented 3 years ago

I hit this same error, although it was for the express request handler and not angular. Basically I was calling next with a pojo instead of an Error object. I do some fun things in my own express error handler to turn it into something of interest, but because the sentryio request middleware goes first, it didn't get this marshalled error.

In the end, created a utility function that would turn whatever was provided into something more reasonable:

export const handleMaybeError = err => {
  if (err instanceof Error) return err
  const newErr = new Error(err.message || 'unexpected')
  for (const [key, value] of Object.entries(err)) {
    newErr[key] = value
  }
  return newErr
}

export const someController = (req, res, next) => {
  try {
    await handleResponse(req, res)
  } catch (err) {
    next(handleMaybeError(err))
  }
}

This muxes up the stack traces I think, but really if it was a pojo passed into this, there was no stack trace anyway.

In our instance, before this was fixed, the vast majority of events reported were for this particular issue and because it's a long-running node web server, breadcrumbs are near useless -- and all the errors get lumped into this one event type.

doctorconceptual commented 3 years ago

const exception = error.error || error.message || error.originalError || error;

this will evaluate to true/false

PhilippMeissner commented 3 years ago

const exception = error.error || error.message || error.originalError || error;

this will evaluate to true/false

Unlikely, while possible. It all comes down to what "error" you send down the stream. But after-all, this should include some kind of useful information.

jacquesdev commented 3 years ago

We are currently using the following to ignore it.

Sentry.init({
  ignoreErrors: [
    'Non-Error exception captured'
  ]
});

Why isn't this option documented anywhere?

kamilogorek commented 3 years ago

@jacquesdev https://docs.sentry.io/platforms/javascript/configuration/filtering/#decluttering-sentry