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

[PLAT-5357] fix(react-native): Ensure plugin usage does not crash debugger #1250

Merged
merged 3 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all 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

- (react-native): Ensure plugin usage is compatible with running an app in a remote debugger [#1250](https://github.com/bugsnag/bugsnag-js/pull/1250)

## v7.6.0 (2020-01-18)

As of 7.6.0 the monorepo contains `@bugsnag/react-native-cli`, a new command line tool to help set up Bugsnag in React Native projects.
Expand Down
60 changes: 38 additions & 22 deletions packages/react-native/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const NativeClient = NativeModules.BugsnagReactNative
const REMOTE_DEBUGGING_WARNING = `Bugsnag cannot initialize synchronously when running in the remote debugger.

Error reporting is still supported, but synchronous calls after Bugsnag.start() will no-op. This means Bugsnag.leaveBreadcrumb(), Bugsnag.setUser() and all other methods will only begin to work after a short delay.
Plugins are also affected. Synchronous calls to Bugsnag.getPlugin() can be used as normal, but plugins may not report errors and set contextual data correctly in the debugger.

This only affects the remote debugger. Execution of JS in the normal way (on the device) is not affected.`

Expand Down Expand Up @@ -97,36 +98,51 @@ const Bugsnag = {
Bugsnag._client._logger.warn('Bugsnag.start() was called more than once. Ignoring.')
return Bugsnag._client
}

if (!isDebuggingRemotely) {
Bugsnag._client = createClient(opts)
return Bugsnag._client
} else {
console.warn(REMOTE_DEBUGGING_WARNING)
let initialised = false
const stubClient = {}
CLIENT_METHODS.reduce((accum, m) => {
stubClient[m] = new Proxy(() => {
console.warn(
`This call to Bugsnag.${m}() is a no-op because remote debugging is enabled and Bugsnag has not yet initialized`
)
}, {
apply: (target, thisArg, argumentsList) => {
if (!initialised) return Reflect.apply(target, thisArg, argumentsList)
return Reflect.apply(Bugsnag._client[m], Bugsnag._client, argumentsList)
}

console.warn(REMOTE_DEBUGGING_WARNING)

let initialised = false

const stubSchema = { ...schema }
// remove the api key from the schema so it doesn't get validated – we know we don't
// have one at this point, and the only other alternative is to use a fake (but valid) one
delete stubSchema.apiKey
const stubClient = new Client({
...opts,
autoTrackSessions: false,
autoDetectErrors: false,
enabledBreadcrumbTypes: []
}, stubSchema, internalPlugins, { name, version, url })

CLIENT_METHODS.forEach((m) => {
if (/^_/.test(m)) return
stubClient[m] = new Proxy(stubClient[m], {
apply: (target, thisArg, args) => {
if (!initialised) {
console.log(`Synchronous call to Bugsnag.${m}()`)
imjoehaines marked this conversation as resolved.
Show resolved Hide resolved
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 changed this to a log rather than a warn, so that it doesn't pop up in the app's yellowbox UI.

}
})
})
Bugsnag._client = stubClient
createClientAsync(opts).then(client => {
initialised = true
Bugsnag._client = client
return Reflect.apply(target, thisArg, args)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this still be applying to Bugsnag._client if it has initialised?

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 it has to carry being functional because plugins created and obtained synchronously still have a reference to that stubbed client. If we stopped apply()ing and no-op'ing, it could fail due to a plugin expecting it to return something.

}
})
return stubClient
}
})

Bugsnag._client = stubClient

createClientAsync(opts).then(client => {
initialised = true
Bugsnag._client = client
})

return stubClient
}
}

CLIENT_METHODS.map((m) => {
CLIENT_METHODS.forEach((m) => {
if (/^_/.test(m)) return
Bugsnag[m] = function () {
if (!Bugsnag._client) return console.warn(`Bugsnag.${m}() was called before Bugsnag.start()`)
Expand Down