feat(utils): Backfill stack trace on fetch errors if it is missing #12389
Conversation
|
@AbhiPrasad is this a very stupid idea? |
AbhiPrasad
left a comment
AbhiPrasad
left a comment
There was a problem hiding this comment.
This feels like a really good idea, if we can make this work we solve soooo many issues for users.
We should test this on safari first, I know they have the worst stacktraces for fetch.
| // it means the error, that was caused by your fetch call did not | ||
| // have a stack trace, so the SDK backfilled the stack trace so | ||
| // you can see which fetch call failed. | ||
| error.stack = new Error(error.message).stack; |
There was a problem hiding this comment.
do we need to trim the frames here? I think we can set framesToPop to do this (error.framesToPop = 1).
size-limit report 📦
|
Do you know how to produce "failed to fetch" errors? |
Just add 
|
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
This looks like a good idea that's worth a try! When we release this let's check in the docs project if this changes anything (given we also get some of these stacktrace-less failed to fetch errors in docs).
| const envelopes = await getMultipleSentryEnvelopeRequests<Event>(page, 4, { url, timeout: 10000 }); | ||
| const tracingEvent = envelopes.find(event => event.type === 'transaction')!; // last envelope contains tracing data on all browsers |
There was a problem hiding this comment.
This, admittedly, is a very sketchy test fix. It appears that the failed to fetch errors were previously not reported but now they are. I think this is more correct, so I am fine with it, but if any of @AbhiPrasad or @Lms24 oppose lmk.
AbhiPrasad
left a comment
AbhiPrasad
left a comment
There was a problem hiding this comment.
could we add an e2e test? Also a screenshot of an example failed to fetch error in the sentry UI to this PR?
I think the test fix seems reasonable to me.
|
Ok so I went ahead and attempted to add a browser integration test. I quickly found out that unless we add this change we are not recording errors in Chrome for a failing call like Also, on Safari the fetch call from above will cause a |
lforst
dismissed
AbhiPrasad’s stale review
discussed offline: problems addressed
This is a lowly attempt at fixing stacktrace-less
failed to fetcherrors that have been plagueing users for years.Note that I did not verify whether this actually works, since we haven't figured out yet how to actually reproduce these errors.
Unintentionally fixes a stacktrace-less safari error:
(before)
(after)
