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

[v8] Fix AWS Lambda plugin not waiting for Event requests when used with server plugins #2120

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

imjoehaines
Copy link
Contributor

Goal

In v8 we moved the responsibility for cloning a request-scoped client from the session plugin to the server plugins (express, Koa & restify). However, we missed that the AWS Lambda plugin still expects the session plugin to clone the clients when using a server integration:

// clone the client when startSession is called if a server plugin is loaded
client._sessionDelegate.startSession = function (client, session) {
const maybeCloned = isServerPluginLoaded(client) ? clone(client) : client
return oldStartSession(maybeCloned, session)
}

The in-flight plugin (used by the AWS Lambda plugin to wait for outstanding requests to finish) relies on monkey patching the Client#notify method and this patch has to be reapplied to cloned clients or it won't be able to track event requests anymore:

// when a client is cloned, make sure to patch the clone's notify method too
// we don't need to patch delivery when a client is cloned because the
// original client's delivery method will be copied over to the clone
clone.registerCallback(patchNotify)

In v7 this works because the AWS Lambda plugin has a copy of the clone-client.js file from @bugsnag/core in its built bundle and is responsible for both registering the on clone callback and cloning the client. Now that the Express plugin is cloning the client, the AWS Lambda plugin registers its on clone callback on a different copy of clone-client.js than the one used by the Express plugin to do the cloning. This means the newly cloned request client will not have the notify monkey patch applied and therefore any Events it sends cannot be tracked

Given that these plugins run in a node environment, we don't actually need to bundle them at all — it was primarily done to run them through babel for old Node versions but that's no longer necessary in v8, which has a minimum Node version of v12

Therefore the fix is as simple as:

  1. remove the AWS Lambda session handler and use the browser plugin directly (essentially reverting Clone clients in AWS Lambda session implementation when a server plugin is used #1887)
  2. remove the build steps from the AWS Lambda plugin and the server plugins

We may also want to look at removing the build steps from all other plugins that are exclusively used in Node (browser plugins will still need to be bundled) to stop this kind of bug from happening again

We no longer need to clone the client in the AWS Lambda session delegate
as it's handled by the server plugins in v8
Copy link

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 44.74 kB 13.64 kB
After 44.74 kB 13.64 kB
± No change No change

code coverage diff

<temporarily disabled>

Generated by 🚫 dangerJS against 2f6cd7b

@imjoehaines imjoehaines merged commit 07155ec into integration/v8 Apr 16, 2024
66 checks passed
@imjoehaines imjoehaines deleted the fix-aws-lambda-with-express branch April 16, 2024 10:52
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.

2 participants