automatic console breadcrumbs for node (bugsnag-js v8) #2107
Conversation
| // if we are in an async context, use the client from that context | ||
| const c = client._clientContext && client._clientContext.getStore() ? client._clientContext.getStore() : client |
There was a problem hiding this comment.
Would this be something we have to do in every plugin used in the node notifier now? If so it seems like we should make this automatic so that the plugins themselves don't need to change, otherwise we're inviting bugs where things are using the wrong client
packages/node/src/notifier.js
Outdated
| @@ -53,6 +53,22 @@ const Bugsnag = { | |||
|
|
|||
| const bugsnag = new Client(opts, schema, internalPlugins, { name, version, url }) | |||
|
|
|||
| Object.keys(Client.prototype).forEach((m) => { | |||
| if (/^_/.test(m)) return | |||
There was a problem hiding this comment.
I'm not sure that this is right — wouldn't we want to proxy the call even if it's a "private" method?
There was a problem hiding this comment.
Yes, this needs to apply to all method calls so that it's consistently applied to the same client e.g. if a public method then calls a private method they need directing to the same place
| @@ -186,7 +186,6 @@ class Client { | |||
| // stuff like __proto__ etc. (only store the result if the plugin had a | |||
| // name) | |||
| if (plugin.name) this._plugins[`~${plugin.name}~`] = result | |||
| return this | |||
There was a problem hiding this comment.
Does this need to be removed?
There was a problem hiding this comment.
It doesn't but I thought it worth removing since we're doing a major version change and it's not used
Co-authored-by: Joe Haines <hello@joehaines.co.uk>
Goal
Design
leaveBreadcrumbmethod on the client clone stored in the async context, if it existsChangeset
Testing