Merge pull request #2660 from bugsnag/hotfix/xhr-handle-reponse

[HOTFIX] Ensure XMLHttpRequest response types are handled gracefully

Author: gingerbenw

.buildkite/basic/expo-pipeline.yml

@@ -25,14 +25,3 @@ steps:
           BUGSNAG_JS_COMMIT: "${BUILDKITE_COMMIT}"
           # a branch name that's safe to use as a docker cache identifier
           BUGSNAG_JS_CACHE_SAFE_BRANCH_NAME: "${BRANCH_NAME}"
-
-    - label: "@bugsnag/expo v52/next"
-      depends_on: "publish-js"
-      trigger: "bugsnag-expo"
-      build:
-        branch: "v52/next"
-        env:
-          BUGSNAG_JS_BRANCH: "${BUILDKITE_BRANCH}"
-          BUGSNAG_JS_COMMIT: "${BUILDKITE_COMMIT}"
-          # a branch name that's safe to use as a docker cache identifier
-          BUGSNAG_JS_CACHE_SAFE_BRANCH_NAME: "${BRANCH_NAME}"

dockerfiles/Dockerfile.browser

@@ -55,7 +55,7 @@ RUN find . -name package.json -type f -mindepth 2 -maxdepth 3 ! -path "./node_mo
 RUN rm -fr **/*/node_modules/
 
 # The maze-runner browser tests (W3C protocol)
-FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v10-cli AS browser-maze-runner
+FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:v10.10.1-cli AS browser-maze-runner
 
 COPY --from=browser-feature-builder /app/test/browser /app/test/browser/
 WORKDIR /app/test/browser

dockerfiles/Dockerfile.node

@@ -1,5 +1,5 @@
 # CI test image for unit/lint/type tests
-FROM node:18-alpine@sha256:974afb6cbc0314dc6502b14243b8a39fbb2d04d975e9059dd066be3e274fbb25 as node-feature-builder
+FROM node:18-alpine@sha256:974afb6cbc0314dc6502b14243b8a39fbb2d04d975e9059dd066be3e274fbb25 AS node-feature-builder
 
 RUN apk add --update bash python3 make gcc g++ musl-dev xvfb-run curl
 
@@ -22,7 +22,7 @@ RUN npm pack --verbose packages/plugin-restify/
 RUN npm pack --verbose packages/plugin-hono/
 
 # The maze-runner node tests
-FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v10-cli as node-maze-runner
+FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:v10.10.1-cli AS node-maze-runner
 WORKDIR /app/
 COPY packages/node/ .
 COPY test/node/features test/node/features

packages/plugin-network-instrumentation/README.md

@@ -20,7 +20,8 @@ Bugsnag.start({
   apiKey: 'YOUR_API_KEY_HERE',
   plugins: [BugsnagPluginNetworkInstrumentation({
     httpErrorCodes = [400, 401, { min: 450: max 499 }], // Status codes to report as errors
-    maxRequestSize = 20_000,                            // Truncate the request and response body over this size (in kb) 
+    maxRequestSize = 20_000,                            // Truncate the request body over this size (in bytes) defaults to 0 (nothing is captured)
+    maxResponseSize = 20_000,                           // Truncate the response body over this size (in bytes) defaults to 0 (nothing is captured)
     onHttpError: ({ request, response }) => {
       request.headers['x-custom-header'] = 'value'      // Modify any request values before sending
       response.body = 'custom body'                     // Modify any response values before sending

packages/plugin-network-instrumentation/network-instrumentation.js

@@ -10,7 +10,8 @@ const shouldCaptureStatusCode = require('./lib/should-capture-status-code')
 const truncate = require('./lib/truncate')
 
 const DEFAULT_HTTP_ERROR_CODES = [{ min: 400, max: 599 }]
-const DEFAULT_MAX_REQUEST_SIZE = 5000
+const DEFAULT_MAX_RESPONSE_SIZE = 0
+const DEFAULT_MAX_REQUEST_SIZE = 0
 
 /**
  * Creates the HTTP errors plugin with configuration
@@ -24,6 +25,7 @@ const DEFAULT_MAX_REQUEST_SIZE = 5000
 module.exports = (config = {}, global = window) => {
   const {
     httpErrorCodes = DEFAULT_HTTP_ERROR_CODES,
+    maxResponseSize = DEFAULT_MAX_RESPONSE_SIZE,
     maxRequestSize = DEFAULT_MAX_REQUEST_SIZE,
     onHttpError
   } = config
@@ -92,9 +94,7 @@ module.exports = (config = {}, global = window) => {
             url: startContext.url,
             httpMethod: startContext.method,
             headers: startContext.headers,
-            params: requestParams,
-            body: startContext.body,
-            bodyLength: startContext.body ? startContext.body.length : undefined
+            params: requestParams
           }
           const responseObj = {
             statusCode: endContext.status,
@@ -114,13 +114,15 @@ module.exports = (config = {}, global = window) => {
           }
 
           // Truncate request body
-          if (requestObj.body) {
-            requestObj.body = truncate(requestObj.body, maxRequestSize)
+          if (maxRequestSize > 0 && startContext.body) {
+            requestObj.body = truncate(startContext.body, maxRequestSize)
+            requestObj.bodyLength = startContext.body.length
           }
 
           // Truncate response body - XHR only
-          if (responseObj.body) {
-            responseObj.body = truncate(responseObj.body, maxRequestSize)
+          if (maxResponseSize > 0 && endContext.body) {
+            responseObj.body = truncate(endContext.body, maxResponseSize)
+            responseObj.bodyLength = endContext.body.length
           }
 
           // Strip query parameters from URL

packages/plugin-network-instrumentation/test/network-instrumentation-xhr.test.ts

@@ -20,9 +20,8 @@ describe('plugin-network-instrumentation', () => {
       status: number | null
       statusText: string
       responseURL: string
-      response: string
-      responseText: string
-      responseType: string
+      response: typeof XMLHttpRequest.prototype.response
+      responseType: typeof XMLHttpRequest.prototype.responseType
       _method: string
       _url: string
       _requestHeaders: Headers
@@ -34,7 +33,6 @@ describe('plugin-network-instrumentation', () => {
         this.statusText = ''
         this.responseURL = ''
         this.response = ''
-        this.responseText = ''
         this.responseType = ''
         this._method = 'GET'
         this._url = ''
@@ -102,7 +100,9 @@ describe('plugin-network-instrumentation', () => {
       const notifyCallbacks: Event[] = []
 
       plugin = createPlugin({
-        httpErrorCodes: { min: 400, max: 499 }
+        httpErrorCodes: { min: 400, max: 499 },
+        maxRequestSize: 1000,
+        maxResponseSize: 1000
       })
 
       const client = new Client({ apiKey: 'api_key', plugins: [plugin] })
@@ -114,7 +114,7 @@ describe('plugin-network-instrumentation', () => {
       xhr.statusText = 'Not Found'
       xhr.responseURL = 'https://api.example.com/users/123'
       xhr.response = '{"error": "User not found", "code": "USER_NOT_FOUND"}'
-      xhr.responseText = '{"error": "User not found", "code": "USER_NOT_FOUND"}'
+      xhr.responseType = 'json'
 
       // Simulate an XHR request
       xhr.open('POST', 'https://api.example.com/users/123')
@@ -143,22 +143,22 @@ describe('plugin-network-instrumentation', () => {
       expect(event.request.httpMethod).toBe('POST')
       expect(event.request.body).toBe(requestBody)
       expect(event.request.bodyLength).toBe(requestBody.length)
-      // expect(event.request.headers?.['content-type']).toBe('application/json')
+      expect(event.request.headers).toStrictEqual({ 'Content-Type': 'application/json' })
 
       // Verify response metadata including body
       expect(event.response.statusCode).toBe(404)
       expect(event.response.headers['content-type']).toBe('application/json')
       expect(event.response.headers['content-length']).toBe('45')
-      expect(event.response.body).toBe('{"error": "User not found", "code": "USER_NOT_FOUND"}')
-      expect(event.response.bodyLength).toBe(xhr.responseText.length)
+      expect(event.response.body).toBe(JSON.stringify(xhr.response))
+      expect(event.response.bodyLength).toBe(JSON.stringify(xhr.response).length)
     })
 
-    it('should truncate XHR response body when it exceeds maxRequestSize', async () => {
+    it('should truncate XHR response body when it exceeds maxResponseSize', async () => {
       const notifyCallbacks: Event[] = []
 
       plugin = createPlugin({
         httpErrorCodes: { min: 400, max: 499 },
-        maxRequestSize: 20
+        maxResponseSize: 20
       })
 
       const client = new Client({ apiKey: 'api_key', plugins: [plugin] })
@@ -171,7 +171,6 @@ describe('plugin-network-instrumentation', () => {
       xhr.statusText = 'Internal Server Error'
       xhr.responseURL = 'https://api.example.com/error'
       xhr.response = largeResponseBody
-      xhr.responseText = largeResponseBody
 
       xhr.open('GET', 'https://api.example.com/error')
       xhr.send()
@@ -225,7 +224,6 @@ describe('plugin-network-instrumentation', () => {
       xhr.status = 403
       xhr.statusText = 'Forbidden'
       xhr.response = 'Forbidden'
-      xhr.responseText = 'Forbidden'
 
       xhr.open('GET', 'https://api.example.com/data?userId=42')
       xhr.send()

packages/plugin-network-instrumentation/test/network-instrumentation.test.ts

@@ -282,7 +282,7 @@ describe('plugin-network-instrumentation', () => {
       expect(requestMetadata.bodyLength).toBe(100)
     })
 
-    it('should use default maxRequestSize of 5000 when not specified', async () => {
+    it('should use default maxRequestSize of 0 when not specified', async () => {
       const notifyCallbacks: Event[] = []
 
       plugin = createPlugin({
@@ -311,8 +311,8 @@ describe('plugin-network-instrumentation', () => {
       const event = notifyCallbacks[0]
       const requestMetadata = event.request
 
-      expect(requestMetadata.body.length).toBeLessThanOrEqual(5000)
-      expect(requestMetadata.bodyLength).toBe(10000)
+      expect(requestMetadata.body).toBeUndefined()
+      expect(requestMetadata.bodyLength).toBeUndefined()
     })
   })
 
@@ -528,7 +528,9 @@ describe('plugin-network-instrumentation', () => {
       const notifyCallbacks: Event[] = []
 
       plugin = createPlugin({
-        httpErrorCodes: { min: 400, max: 499 }
+        httpErrorCodes: { min: 400, max: 499 },
+        maxRequestSize: 1000,
+        maxResponseSize: 1000
       })
 
       const client = new Client({ apiKey: 'api_key', plugins: [plugin] })

packages/plugin-network-instrumentation/types/bugsnag-plugin-network-instrumentation.d.ts

@@ -25,11 +25,21 @@ export interface BugsnagPluginHttpErrorsConfiguration {
   httpErrorCodes?: number | HttpErrorRange | Array<number | HttpErrorRange>
 
   /**
-   * Maximum size of the request body to capture (in characters)
-   * @default 5000
+   * Maximum size in bytes of the request body to capture
+   * Disabled as default
+   * @default 0
    */
   maxRequestSize?: number
 
+  /**
+   * Maximum size in bytes of the response body to capture
+   * Does not capture streaming responses, such as a
+   * fetch request with a ReadableStream body
+   * Disabled as default
+   * @default 0
+   */
+  maxResponseSize?: number
+
   /**
    * Callback function to intercept HTTP errors before they are reported.
    * Return false to prevent the error from being reported.

packages/request-tracker/lib/xhr-response-parser.js

@@ -0,0 +1,28 @@
+/**
+ * Receives the XHR response and parses it based on the responseType
+ * @param {XMLHttpRequest} xhr The XHR instance
+ * @returns {string | undefined} The parsed response
+ */
+module.exports = function xhrResponseParser ({ response, responseType }) {
+  if (response === null || response === undefined) {
+    return undefined
+  }
+
+  switch (responseType) {
+    case 'arraybuffer':
+    case 'blob':
+      return '[Binary Data]'
+    case 'document':
+      return '[Document]'
+    case 'json':
+      try {
+        return JSON.stringify(response)
+      } catch (e) {
+        return '[Unserializable JSON]'
+      }
+    case 'text':
+    case '':
+    default:
+      return String(response)
+  }
+}

packages/request-tracker/lib/xhr-tracker.js

@@ -1,5 +1,6 @@
 const RequestTracker = require('./request-tracker')
 const xhrHeaderStringToObject = require('./xhr-header-string-to-object')
+const xhrResponseParser = require('./xhr-response-parser')
 
 /**
  * Create XHR request tracker with singleton pattern
@@ -72,7 +73,7 @@ function createXhrTracker (global, options = {}) {
             status: this.status,
             state: 'success',
             headers: getResponseHeaders(),
-            body: this.responseText
+            body: xhrResponseParser(this)
           })
         }
 
@@ -81,7 +82,7 @@ function createXhrTracker (global, options = {}) {
             endTime: Date.now(),
             state: 'error',
             headers: getResponseHeaders(),
-            body: this.responseText
+            body: xhrResponseParser(this)
           })
         }