Skip to content
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

Prevent error when running inline script plugin on empty stacktraces #563

Merged
merged 5 commits into from
Jun 25, 2019

Conversation

bengourley
Copy link
Contributor

@bengourley bengourley commented Jun 24, 2019

In the case of an error without a stacktrace, the beforeSend implemented by @bugsnag/plugin-inline-script-content would throw an error (see #559).

Since the logic that runs the list of beforeSend callbacks tolerates errors, the effect of this was pretty minimal – just some undesirable error logs output.

The fix supplied by @andvla (thanks 🙏) was tweaked so that we won't attempt to look for .lineNumber on any falsey value for stacktrace[0].

@@ -74,7 +74,7 @@ module.exports = {
}

// only attempt to grab some surrounding code if we have a line number
if (frame.lineNumber === undefined) return
if (!frame || frame.lineNumber === undefined) return
Copy link
Contributor

Choose a reason for hiding this comment

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

oh, I did if (frame === undefined || frame.lineNumber === undefined) return for consistency but in case you are checking for !frame, might change the second condition to do the same? so...:

Suggested change
if (!frame || frame.lineNumber === undefined) return
if (!frame || !frame.lineNumber) return

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left it as === undefined because 0 is a valid number which evaluates as falsey. However, 0 isn't really a valid value for line number so arguably this is unnecessary.

Copy link

@sazap10 sazap10 left a comment

Choose a reason for hiding this comment

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

SFTM

@bengourley bengourley merged commit c506250 into next Jun 25, 2019
@bengourley bengourley deleted the empty-stacktrace-inline-script-bug branch June 25, 2019 12:29
@andvla
Copy link
Contributor

andvla commented Jun 25, 2019

@bengourley, could you please release that fix within the next version? Would like to start using it. Thanks 😉

@bengourley
Copy link
Contributor Author

Sure, we've pencilled in a release at some point this week.

Note that this isn't causing any issues or lost reports – the beforeSend callback runner tolerates the error so its just a log message, albeit undesirable!

@andvla
Copy link
Contributor

andvla commented Jun 26, 2019

@bengourley After updating to 6.3.1 keep on getting this error
Screenshot 2019-06-26 at 14 55 57

And this is coming from @bugsnag/browser/dist/bugsnag.js line 2152, not sure if there's a way for me to fix it.

@andvla
Copy link
Contributor

andvla commented Jun 26, 2019

I guess bugsnag/browser needs another release as well

@bengourley
Copy link
Contributor Author

@andvla this is now released in v6.3.2

@andvla
Copy link
Contributor

andvla commented Jul 1, 2019

@bengourley perfect, thanks!

@byzyk
Copy link

byzyk commented Jul 7, 2019

Hi @bengourley! Was wondering why 6.3.2 is not available on NPM and if this is going to be published at some point?

Thanks for the fix btw 🎉

@bengourley
Copy link
Contributor Author

It should be available on npm: https://www.npmjs.com/package/@bugsnag/js/v/6.3.2

@byzyk
Copy link

byzyk commented Jul 8, 2019

Aw sorry my bad was looking at react plugin 😃

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