Skip to content

fix: crash on iterating "delayedCalls" #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 30, 2020
Merged

fix: crash on iterating "delayedCalls" #219

merged 2 commits into from
Jul 30, 2020

Conversation

rchl
Copy link
Member

@rchl rchl commented Jul 29, 2020

It looks like the code allowed "loadSentry()" to run more than once and
the first run would unset "delayedCalls" causing second run to crash on
trying to iterate it.

It looks like the code allowed "loadSentry()" to run more than once and
the first run would unset "delayedCalls" causing second run to crash on
trying to iterate it.
@rchl rchl requested a review from pimlie July 29, 2020 13:05
@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #219 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #219   +/-   ##
=======================================
  Coverage   55.55%   55.55%           
=======================================
  Files           1        1           
  Lines          27       27           
  Branches        8        8           
=======================================
  Hits           15       15           
  Misses          9        9           
  Partials        3        3           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fb726d...df933af. Read the comment docs.

@rchl
Copy link
Member Author

rchl commented Jul 29, 2020

Hopefully handles this issue: https://sentry.io/share/issue/2c22b5e9282a4a7986d910433e84518f/#exception

There is quite a lot of extra checking in this PR that should stop the code running twice.

But I can't quite see how this linked error was possible as for loadSentry to trigger twice (with default lazy settings), we would need to add another onNuxtReady and while there was code that did that, that function could have only been triggered by onNuxtReady hook itself so window.$nuxt should have existed already...

@rchl
Copy link
Member Author

rchl commented Jul 29, 2020

Hmm, I'm not even sure if that error is about calling forEach on undefined or about calling Sentry API from forEach. The error message is pretty poor on safari webview.

@rchl
Copy link
Member Author

rchl commented Jul 30, 2020

Will get this in and see if it still reproduces.

@rchl rchl merged commit d32b6e5 into master Jul 30, 2020
@rchl rchl deleted the fix/multi-init-crash branch July 30, 2020 07:25
rchl added a commit that referenced this pull request Jul 30, 2020
* fix: crash on iterating "delayedCalls"

It looks like the code allowed "loadSentry()" to run more than once and
the first run would unset "delayedCalls" causing the second run to crash on
trying to iterate it.

* Null-check vm as it might not exist
@rchl
Copy link
Member Author

rchl commented Jul 30, 2020

That didn't fix it...

I will have to inject some debugging code to try to figure it out.
I can only check it on production though so that makes it quite tricky to debug...

@rchl
Copy link
Member Author

rchl commented Jul 30, 2020

Just realized that this is expected. It's reposting the error caught through error handler.
The part that made me confused is the stack trace pointing at the sentry plugin. I could probably repost from a timeout to hopefully lose the stack as it's not relevant in those cases.

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.

None yet

1 participant