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

fix(plugin-koa) add request body if present #1292

Merged
merged 13 commits into from
Mar 16, 2021
Merged

Conversation

djskinner
Copy link
Contributor

@djskinner djskinner commented Feb 12, 2021

Goal

Include the request body when ctx.request.body is present, for example, when using the koa-bodyparser middleware.

Closes #1183

Design

Even if the request body was being included in request and metadata it wouldn't have been included because those values were being determined when the requestHandler 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 and cookies were not added as part of this change as it was not clear which middleware we should support (whereas koa-bodyparser seems to be the de-facto standard for koa body parsing)

Changeset

  • Add body: ctx.request.body to extractRequestInfo
  • Include body in request in getRequestAndMetadataFromCtx
  • Include httpVersion in request in getRequestAndMetadataFromCtx. This is an unrelated change but further brings the plugin into compliance with the spec.
  • Call 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.

@github-actions
Copy link

github-actions bot commented Feb 12, 2021

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 40.76 kB 12.57 kB
After 40.76 kB 12.57 kB
± No change No change

code coverage diff

Coverage values did not change👌.

Total:

Lines Branches Functions Statements
0%(+0%) 0%(+0%) 0%(+0%) 0%(+0%)

Generated by 🚫 dangerJS against 5a9f3a5

* 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
@djskinner djskinner marked this pull request as ready for review February 19, 2021 18:03
test/node/features/koa.feature Show resolved Hide resolved
packages/plugin-koa/src/koa.js Show resolved Hide resolved
* 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
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.

plugin-koa: body not showing up in request tab
3 participants