function isSameStacktrace() is thrown with TypeError: Cannot read property 'frames' of undefined

[zivl]

Inside raven.js there is a logic in isSameStacktrace function to determine whether the current error is the same as the previous one (in a certain time frame).

I have something like the following code:

promise.catch(e => Raven.catchException(e))

and I get this report in Sentry:

TypeError: Cannot ready property 'frames' of undefined
(line 324 is highlighted: var frames1 = stack1.frames;)

I would expect to have another "null-check" before accessing this property.

I'm using "raven-js": "^3.12.1", and get this report for different browsers and different operating systems.

Let me know if you find this issue relevant and I'll be happy to submit a fix
Cheers,

[kamilogorek]

Hey @zivl, can you provide a minimal repr-case I could use?

The code you mentioned is used here:

if (current.stacktrace || last.stacktrace) {
  return isSameStacktrace(current.stacktrace, last.stacktrace);
}

which makes sure that we have at least one object to work with.

Then isSameStacktrace checks for similar thing again here:

if (isOnlyOneTruthy(stack1, stack2)) return false;

I'm not sure how we'd end up with stack1 === null.

[zivl]

@kamilogorek
Unfortunately, I don't have a specific repr-case scenario. I know it makes your life harder a bit.
Anyway, in those exact lines that you've mentioned above, I think this should be the change

if (current.stacktrace && last.stacktrace) { // here we must sure we have both stacktrace(s)
  return isSameStacktrace(current.stacktrace, last.stacktrace);
}

The fact that we don't know how we end up with null does not really matter. As framework developer, I know that I might get unexpected input so I should either check for it and handle or throw exception, but my framework's code must not fail.

WDYT?

[kamilogorek]

If we change it to &&, then it'll always return true for payloads where one thing has stacktrace and second does not (eg. captureException was last event, but now we compare it to captureMessage)

Current code:

if (current.stacktrace || last.stacktrace) {
  return isSameStacktrace(current.stacktrace, last.stacktrace);
} else if (current.exception || last.exception) {
  return isSameException(current.exception, last.exception);
}    

return true;

A reason why I want to see some repro-case, is that this clause

if (isOnlyOneTruthy(stack1, stack2)) return false;

Already guards for non-truthy imputs, therefore it should never be possible to get null.frames call.

You can find all tests for _isRepeatData starting line 3325 in tests/raven.test.js to see what has to pass. Using && breaks some cases.

[zivl]

I saw the tests in raven.test.js, though, to my humble opinion, I think the default for _isRepeatData should return false. In case of true, I might miss some errors just because they have not pass in one of the conditions. Isn't the desired behavior should prefer to send anyway ? I rather have a few more errors to check but at least I know about them (and later on I can provide even better checks for duplicate conditions)

Anyway, I agree that after this line isOnlyOneTruthy, the rest of the code should not be run if either of the stacks is undefinded

[zivl]

@kamilogorek
The thing is that return isSameStacktrace(ex1.stacktrace, ex2.stacktrace); that is being called in isSameException does not have the same protection like 'isSameStacktrace'.

The condition current.exception || last.exception doesn't garrantee that exception has a stack trace. In the current situation, both last.exception.stacktrace and current.exception.stacktrace are undefined - hence the script fails

[kamilogorek]

Resolved in #1150