-
Notifications
You must be signed in to change notification settings - Fork 251
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
Clone clients in AWS Lambda session implementation when a server plugin is used #1887
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a server plugin (e.g. Express) is loaded, we need to clone the client returned from `startSession` in order for them to behave as expected We can't unconditionally clone clients as this causes events and sessions to be associated with different clients, breaking the link between the two
imjoehaines
commented
Dec 8, 2022
@@ -83,5 +83,4 @@ Scenario: promise rejections are reported when using serverless-express | |||
Then the session is valid for the session reporting API version "1" for the "Bugsnag Node" notifier | |||
And the session "id" is not null | |||
And the session "startedAt" is a timestamp | |||
And the event "session.events.handled" equals 0 | |||
And the event "session.events.unhandled" equals 1 | |||
And the event "session.events" is null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This promise rejection isn't associated with a session anymore, because the session is started by the Express plugin in the request but an unhandled promise rejection is caught globally and therefore by a different client with no knowledge of the session
djskinner
reviewed
Dec 8, 2022
djskinner
approved these changes
Dec 8, 2022
imjoehaines
force-pushed
the
aws-lambda-session-clones
branch
from
December 8, 2022 15:26
629a4d7
to
07b1b9d
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
This PR fixes a conflict between the AWS Lambda plugin and server plugins (Express, Koa & Restify) where the server plugins expect the session delegate to create a cloned client but the AWS Lambda session delegate doesn't do that
This is a bit more complicated than simply cloning the client in the AWS Lambda session delegate — doing so causes unexpected behaviour when not using a server plugin as the client created by
Bugsnag.start
would not have access to the session information and therefore session data doesn't get attributed to events properlySee #1875