Sentry-javascript: Are DOMExceptions handled differently than normal errors?

Created on 5 Feb 2019  ·  19Comments  ·  Source: getsentry/sentry-javascript

Package + Version

  • [x] @sentry/browser

Version:

4.2.1

Description

We have some code in our application that takes advantage of the abortable fetch API. This a native feature in many recent browsers, and there are polyfills for those browsers that don't support it.

The recommended usage is something like this. The idea is that you fire a signal that you've attached to a fetch request, and if the fetch is still pending, it rejects with a DOMException whose .name is AbortError.

Our particular implementation looks like this:

let searchAbortController = null;
const emptyMap = Map();

const makeSearchCall = async (parameters, dispatch) => {
  // Create a new AbortController for the context of this request
  searchAbortController = new AbortController();
  let response;
  try {
    response = await dispatch(apiFetchThunk(
      '/api/endpoint/',
      { signal: searchAbortController.signal },
    ));
  } catch (error) {
    // TODO: figure out why DOMExceptions are occasionally leaking
    //  to Sentry.
    // We don't want to do anything if the error is an abort,
    // since we only abort intentionally
    if (
      (error.name === 'AbortError')
      || (error instanceof DOMException)
    ) {
      return emptyMap;
    }
    throw error;
  }
  // ...
};

Near as I can tell, any DOMExceptions should be swallowed, and that's the behavior on my local dev environment, and that's true even when I've got the Sentry DSN pointed at one of our Sentry projects.

We still see errors come through, however: AbortError: The user aborted a request.. We don't want these to hit Sentry, because we explicitly don't care about them.

My question: is there something about the way that DOMExceptions are created, propagated, and handled that would cause this behavior? Is this expected? If it is, how do we change our application code to get around it?

Question

All 19 comments

They are handled in the same manner as any other errors that bubble up to onerrorhttps://github.com/getsentry/sentry-javascript/blob/89bca28994a0eaab9bc784841872b12a1f4a875c/packages/browser/src/backend.ts#L80-L90

If it is, how do we change our application code to get around it?

You can use our InboundFilter integration documented here – https://docs.sentry.io/platforms/javascript/default-integrations/#inboundfilters

tl;dr

```js
Sentry.init({
dsn: '__YOUR_DSN__',
ignoreErrors: ['AbortError']
});

Gotcha. I knew about ignoreErrors, but my concern is that that's a gliobal ignore, where I was really wanting to make sure that there wasn't something I could do to fix this particular case.

There's also still a possibility that there's a bug in our code. Thank you for your response!

Hey guys! Just found out we're still not getting reports on DOMException's like this one:

Screen Shot 2019-07-08 at 09 58 28

Maybe we need to scope.setLevel for a different one, but as far as I know should default to Error level. Thinking this might be related to https://github.com/getsentry/sentry-javascript/issues/1864

Concept of levels is just important for the UI. Do you have any repro-case I could use to debug this? Or a page where it happens?

@kamilogorek I don't unfortunately, but I can't tell you it was a case where the DOMElement id had long strings with strange & invisible characters (carriage returns, etc), so querySelectorAll was failing. Honestly it's an edge-case, as seems to be a bad practice for setting elements ID's, so we fixed it already.

Anyways, I still think those should be reported to Sentry tho 🙈, even being a DOM API, Found this related links, hope they're helpful:

Anyways, I still think those should be reported to Sentry tho 🙈, even being a DOM API, Found this related links, hope they're helpful:

They should, and if they bubble to error handler they will with the current code. It's hard to tell what went wrong there and why you didn't see anything in your Sentry panel without repro :(

@kamilogorek here you go, I was able to reproduce it, see the console:

https://codesandbox.io/s/react-codesandbox-q76wl

Screen Shot 2019-07-10 at 15 42 44

@benoror cloned your repo, added barebone Sentry SDK and it works just fine 🤔

image

@kamilogorek weird, I tried several times with no luck. I am using create-react-app, and placed the sentry config in src/index.js. May I see your setup?

@benoror I use exactly your code (even exported whole codesandbox :p) and I'm running it with CRA as well using yarn start from package.json above.

import React from 'react'
import ReactDOM from 'react-dom'
import ReactTooltip from 'react-tooltip'
import * as Sentry from '@sentry/browser'

Sentry.init({
  debug: true,
  dsn: 'http://[email protected]/123',
  beforeSend(e) {
    console.log(e)
    return null
  },
})

function App() {
  const malformedId =
    'VDCEP \n Velcade, dexamethasone, Cyclophosomide, Etopside, \nCisplain'
  return (
    <div>
      <span data-tip data-for={malformedId}>
        {' '}
        d(`・∀・)b{' '}
      </span>
      <ReactTooltip id={malformedId} type="error">
        <span>Show happy face</span>
      </ReactTooltip>
    </div>
  )
}

ReactDOM.render(<App />, document.getElementById('root'))

Same setup, except for the beforeSend 🤷‍♂

Just JavaScript stuff I guess :D
Maybe some Chrome Extension is taking over global error handler or something? You may want to add debug/beforeSend with console.log and run it in incognito mode.
Or maybe build a project and use serve to run it and see if it makes any difference.

@kamilogorek yep, I will. Thanks buddy!

Sorry to necro - I can open a new issue if that'd be preferred - but this is still bugging me.

Here's an example of an "unhandled promise rejection" being reported by Sentry, which is separate from the issue that I described when I originally opened this issue: https://sentry.io/share/issue/50c212229be84053826ba2f547876959/ (not sure if you can use this to view the event that I created the share link from - I can add an event ID if that'd be more helpful).

Here's the calling code:

export const getStuff = values => (
  dispatch => (
    dispatch(apiFetchThunk(
      '/api/endpoint/',
      {
        method: 'POST',
        body: JSON.stringify(values),
      },
    )))
    .then(response => {
      if (!response.ok) {
        if (response.status === 404) {
          // TODO: this is awkward and probably should not be nested.
          return response
            .json()
            .then(body => dispatch(setStuff(body.search_url)))
            .then(() => List());
        }
        throw response;
      }
      return response.json();
    })
);

Which does throw and reject in the event of a non-2xx non-404 response. However, the code executing this call is handled by redux-form:

const onSubmit = (values, dispatch, props) => {
  const { thing, otherThing } = props;
  return dispatch(getStuff({
    ...thing.get('details').toJS(),
    otherThing: otherThing.get('id'),
  })
};
// ...
const decorators = [
  reduxForm({
    form: 'form',
    onSubmit,
  }),
  connect(mapStateToProps),
];

My understanding of the way that redux-form works is that rejections in an onSubmit function are swallowed. When I go diving into the redux-form source code, the only thing that looks weird to me in this regard is that they use .then(thenFn, catchFn) syntax in the handler logic and not .then(thenFn).catch(catchFn): https://github.com/erikras/redux-form/blob/master/src/handleSubmit.js#L75-L100 Does this point to a potential problem?

My understanding may be incorrect, but the reason that I'm raising this here and not in the redux-form repo is because we started seeing these unhandled promise rejections on April 9, which was when we upgraded from @sentry/[email protected] to @sentry/[email protected]. Was there something in the v5 major that significantly changed how promises and their rejections are handled?

Was there something in the v5 major that significantly changed how promises and their rejections are handled?

No, not really. However, the issue that you are seing is caused by calling reject or throw with an object/string instead of Error object.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/reject#Description

For debugging purposes and selective error catching, it is useful to make reason an instanceof Error.

Throws and rejections of non-Error instances, prevent us from extracting any stacktrace, as they simply dont contain them.

The issue with that sentry report is less that it doesn't contain a stack trace (it makes sense to me that there wouldn't be one without an instance of an Error), but more that it exists at all. These unhandled rejections are all theoretically handled, and indeed Sentry treated them as though they were prior to version 5.

Is there something different in the way that catch works with Error instances vs a regular javascript object or primitive?

Is there something different in the way that catch works with Error instances vs a regular javascript object or primitive?

No, throw keyword is "in short" just redirecting the value to 1st argument of catch clause.

but more that it exists at all. These unhandled rejections are all theoretically handled, and indeed Sentry treated them as though they were prior to version 5.

There's no magic here, and there has to be a place/reason where/why it's caught and reported 🤔

@kamilogorek thank you for being patient with me. The problem ended up being between the chair and the keyboard. It turns out that redux-form only catches SubmissionErrors (an error type exported by the library), and I had been expecting it to catch all errors.

I'm not sure why these errors weren't getting logged prior to the 4->5 sentry upgrade we did, but it's a moot point at this point.

Thanks for letting me know the details. Cheers!

Was this page helpful?
0 / 5 - 0 ratings