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
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## TBD

### Fixed

- (plugin-inline-script): Ensure inline script content callback doesn't cause error logs when there are no stackframes [#559](https://github.com/bugsnag/bugsnag-js/pull/559) / [#563](https://github.com/bugsnag/bugsnag-js/pull/563)

## 6.3.1 (2019-06-17)

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

frame.code = addSurroundingCode(frame.lineNumber)
})

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { describe, it, expect } = global
const { describe, it, expect, spyOn } = global

const plugin = require('../')

Expand Down Expand Up @@ -157,4 +157,37 @@ Lorem ipsum dolor sit amet.
})
expect(payloads[0].events[0].metaData.script).toBeDefined()
})

it('works when the stacktrace is empty', () => {
const scriptContent = `console.log("EMPTY")`
const document = {
scripts: [ { innerHTML: scriptContent } ],
currentScript: { innerHTML: scriptContent },
documentElement: {
outerHTML: `<p>
Lorem ipsum dolor sit amet.
Lorem ipsum dolor sit amet.
Lorem ipsum dolor sit amet.
</p>
<script>${scriptContent}
</script>
<p>more content</p>`
}
}
const window = { location: { href: 'https://app.bugsnag.com/errors' } }

const client = new Client(VALID_NOTIFIER)
const payloads = []
client.setOptions({ apiKey: 'API_KEY_YEAH' })
client.configure()
client.use(plugin, document, window)

expect(client.config.beforeSend.length).toBe(1)
client.delivery(client => ({ sendReport: (payload) => payloads.push(payload) }))
const spy = spyOn(client._logger, 'error')
client.notify(new Report('EmptyStacktrace', 'Has nothing in it', []))
expect(payloads.length).toEqual(1)
expect(payloads[0].events[0].stacktrace).toEqual([])
expect(spy).toHaveBeenCalledTimes(0)
})
})