-
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
fix(plugin-koa) add request body if present #1292
Merged
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
* next: v7.7.0 chore: update changelog android: alter plugin lookup for bugsnag react-native dep: update bugsnag-android dep to v5.6.2 chore: Update changelog refactor(plugin-inline-script): Assume inline script while the DOM content has not yet loaded test(plugin-inline-script-content): Ensure script content isn't erroneously set when Bugsnag is loaded async'ly fix(plugin-inline-script-content): Check document.readyState in case we missed the interactive event fix(plugin-inline-script-content): Don't assume errors with no stacktrace are inline scripts
imjoehaines
approved these changes
Feb 19, 2021
bengourley
approved these changes
Feb 25, 2021
* next: (58 commits) v7.8.2 chore: update changelog for release Remove setting of bind-address Tests/Node v4: Tidy up Node Gemfile Tests/Node v4: Update MR dependencies latest v4 release Tests/Node v4: Remove debug steps and reinstate pipeline Tests/Node v4: Use correct bind-address syntax Tests/Node v4: Ensure bind-address is set correctly Tests/Node v4: Add extra debugging Tests/Node-MR-V4: Use new steps to set endpoints for test fixtures Tests/Node-MR-V4: Update used MR image Test/Node-MR-V4: Remove all except node pipeline temporarily Test/Node-MR-V4: Initial update to v4 bump android dependency to v5.7.1 deps(react-native): Update bugsnag-cocoa to v6.7.0 Browser bind-address 0.0.0.0 chore: Update changelog fix(expo): Ensure Expo plugins depend on same version of NetInfo package Fix browser tests using removed constant Update to latest MazeRunner ...
* next: Ensure we skip the correct number of frames Update ios vendoring script to match PLAT-5777 requirements Pin RN Navigation dependency versions
bengourley
reviewed
Mar 15, 2021
* next: Update bugsnag-cocoa to v6.7.1
This was referenced Mar 16, 2021
Merged
This was referenced Mar 17, 2021
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
Include the request body when
ctx.request.body
is present, for example, when using thekoa-bodyparser
middleware.Closes #1183
Design
Even if the request body was being included in
request
andmetadata
it wouldn't have been included because those values were being determined when therequestHandler
middleware was run, which would be before any body parsing middleware is added, as per our integration instructions, which suggest adding the Bugsnag middleware first.The changes here are somewhat in line with the way the express plugin works, see #872 with the exception being the body is only added to
event.request
, rather than metadata, which is the preferred approach going forwards in alignment with the notifier spec.httpVersion
is also added for the same reason.params
andcookies
were not added as part of this change as it was not clear which middleware we should support (whereaskoa-bodyparser
seems to be the de-facto standard for koa body parsing)Changeset
body: ctx.request.body
toextractRequestInfo
body
inrequest
ingetRequestAndMetadataFromCtx
httpVersion
inrequest
ingetRequestAndMetadataFromCtx
. This is an unrelated change but further brings the plugin into compliance with the spec.getRequestAndMetadataFromCtx(ctx)
in the onError handler instead of when the middleware is first executed. Add the metadata to the event, not the client as any metadata added to the client during onError is not included in the event.Testing
The changes are covered by an E2E test.