Skip to content

Conversation

@mattrobenolt
Copy link
Contributor

In V8, Error.stack is truncated with a default Error.stackTraceLimit
of 10. So we want to expose this and set it to 50 to capture more.

/cc @dcramer

Also worth noting, that this is probably useless without XHR being the default transport, since this will very easily push us over our querystring limitations.

@dcramer
Copy link
Member

dcramer commented Nov 11, 2015

🎱

@mattrobenolt
Copy link
Contributor Author

@benvinegar I rebased this against master since there were a bunch of conflicts. Wanna take a look now?

@mattrobenolt
Copy link
Contributor Author

I guess a thing to consider, even with POST, is we don't have room for infinite payloads. We only have 100KB to send for the POST body.

So in retrospect, not sure if this is a great idea to go to Infinity. Maybe just increase it to 50?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to modify this? Why is it not sufficient to just slice frames before transmitting?

Note this means that multiple error instances of Raven.js will clobber each other's Error.stackTraceLimit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-reads PR message

Oh, I see re: V8. Hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this only affects V8, and unfortunately, it's a global option.

V8 just defaults to a limiting, 10 frames. We've personally hit an issue with this in Sentry since React generates some long stack traces.

In theory, we'd push all browsers to capture Infinity frames, then slice() out of that, but that's not how reality works. :(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah let's go with 50.

In V8, `Error.stack` is truncated with a default `Error.stackTraceLimit`
of 10. So we want to expose this and set it to Infinity to capture it
all.
We don't have infinite room to send data, we only have 100KB, even for
POST, so we need to be a little bit conservative. 50 should be enough
for most cases. If we need to handle more, we'll need to implement
compression on the client so our packet is less than 100KB like our
other server side clients.
mattrobenolt added a commit that referenced this pull request Jan 8, 2016
Support setting Error.stackTraceLimit automatically
@mattrobenolt mattrobenolt merged commit 2d22b7c into master Jan 8, 2016
@mattrobenolt mattrobenolt deleted the stacktracelimit branch January 8, 2016 22:09
@mattrobenolt mattrobenolt mentioned this pull request Jan 19, 2016
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

Successfully merging this pull request may close these issues.

4 participants