Merge pull request #1823 from bugsnag/plat-8731

remove js payload limit

.buildkite/browser-pipeline.yml

@@ -107,23 +107,6 @@ steps:
         concurrency: 5
         concurrency_group: "bitbar-web"
 
-      - label: ":safari: 16 Browser tests"
-        depends_on: "browser-maze-runner"
-        timeout_in_minutes: 30
-        plugins:
-          docker-compose#v3.9.0:
-            pull: browser-maze-runner
-            run: browser-maze-runner
-            use-aliases: true
-            command:
-              - --farm=bb
-              - --browser=safari_16
-          artifacts#v1.5.0:
-            upload:
-              - "./test/browser/maze_output/failed/**/*"
-        concurrency: 5
-        concurrency_group: "bitbar-web"
-
       #
       # BrowserStack tests
       #
@@ -263,6 +246,23 @@ steps:
         concurrency: 2
         concurrency_group: "browserstack"
 
+      - label: ":safari: 16 Browser tests"
+        depends_on: "browser-maze-runner"
+        timeout_in_minutes: 30
+        plugins:
+          docker-compose#v3.9.0:
+            pull: browser-maze-runner
+            run: browser-maze-runner
+            use-aliases: true
+            command:
+              - --farm=bs
+              - --browser=safari_16
+          artifacts#v1.5.0:
+            upload:
+              - "./test/browser/maze_output/failed/**/*"
+        concurrency: 2
+        concurrency_group: "browserstack"
+
       - label: ":iphone: iOS 10.3 Browser tests"
         depends_on: "browser-maze-runner"
         timeout_in_minutes: 30

CHANGELOG.md

@@ -2,8 +2,9 @@
 
 ## TBD
 
-### Enhancements
+### Changed
 
+- After trimming, attempt to send all event and session payloads, even if believed oversize [#1823](https://github.com/bugsnag/bugsnag-js/pull/1823)
 - (react-native) Update bugsnag-android from v5.28.1 to [v5.28.3](https://github.com/bugsnag/bugsnag-android/blob/master/CHANGELOG.md#5283-2022-11-16)
 
 ### Fixed

packages/core/lib/json-payload.js

@@ -15,13 +15,11 @@ Serialized payload was ${payload.length / 10e5}MB (limit = 1MB)
 metadata was removed`
     }
     payload = jsonStringify(event, null, null, { redactedPaths: EVENT_REDACTION_PATHS, redactedKeys })
-    if (payload.length > 10e5) throw new Error('payload exceeded 1MB limit')
   }
   return payload
 }
 
-module.exports.session = (event, redactedKeys) => {
-  const payload = jsonStringify(event, null, null)
-  if (payload.length > 10e5) throw new Error('payload exceeded 1MB limit')
+module.exports.session = (session, redactedKeys) => {
+  const payload = jsonStringify(session, null, null)
   return payload
 }

packages/core/lib/test/json-payload.test.ts

@@ -0,0 +1,102 @@
+import jsonPayload from '../json-payload'
+
+function makeBigObject () {
+  var big: Record<string, string> = {}
+  var i = 0
+  while (JSON.stringify(big).length < 2 * 10e5) {
+    big['entry' + i] = 'long repetitive string'.repeat(1000)
+    i++
+  }
+  return big
+}
+
+describe('jsonPayload.event', () => {
+  it('safe stringifies the payload and redacts values from certain paths of the supplied keys', () => {
+    expect(jsonPayload.event({
+      api_key: 'd145b8e5afb56516423bc4d605e45442',
+      events: [
+        {
+          errorMessage: 'Failed load tickets',
+          errorClass: 'CheckoutError',
+          user: {
+            name: 'Jim Bug',
+            email: 'jim@bugsnag.com'
+          },
+          request: {
+            api_key: '245b39ebd3cd3992e85bffc81c045924'
+          }
+        }
+      ]
+    }, ['api_key'])).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"[REDACTED]"}}]}')
+  })
+
+  it('strips the metaData of the first event if the payload is too large', () => {
+    const payload = {
+      api_key: 'd145b8e5afb56516423bc4d605e45442',
+      events: [
+        {
+          errorMessage: 'Failed load tickets',
+          errorClass: 'CheckoutError',
+          user: {
+            name: 'Jim Bug',
+            email: 'jim@bugsnag.com'
+          },
+          request: {
+            api_key: '245b39ebd3cd3992e85bffc81c045924'
+          },
+          _metadata: {}
+        }
+      ]
+    }
+
+    payload.events[0]._metadata = { 'big thing': makeBigObject() }
+
+    expect(jsonPayload.event(payload)).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"245b39ebd3cd3992e85bffc81c045924"},"_metadata":{"notifier":"WARNING!\\nSerialized payload was 2.003435MB (limit = 1MB)\\nmetadata was removed"}}]}')
+  })
+
+  it('does not attempt to strip any other data paths from the payload to reduce the size', () => {
+    const payload = {
+      api_key: 'd145b8e5afb56516423bc4d605e45442',
+      events: [
+        {
+          errorMessage: 'Failed load tickets',
+          errorClass: 'CheckoutError',
+          user: {
+            name: 'Jim Bug',
+            email: 'jim@bugsnag.com'
+          },
+          _metadata: {}
+        },
+        {
+          errorMessage: 'Request failed',
+          errorClass: 'APIError',
+          _metadata: {}
+        }
+      ]
+    }
+    payload.events[1]._metadata = { 'big thing': makeBigObject() }
+
+    expect(jsonPayload.event(payload).length).toBeGreaterThan(10e5)
+  })
+})
+
+describe('jsonPayload.session', () => {
+  it('safe stringifies the payload', () => {
+    expect(jsonPayload.session({
+      api_key: 'd145b8e5afb56516423bc4d605e45442',
+      events: [
+        {
+          errorMessage: 'Failed load tickets',
+          errorClass: 'CheckoutError',
+          user: {
+            name: 'Jim Bug',
+            email: 'jim@bugsnag.com'
+          },
+          request: {
+            api_key: '245b39ebd3cd3992e85bffc81c045924'
+          }
+        }
+      ]
+    }, ['api_key'])).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"245b39ebd3cd3992e85bffc81c045924"}}]}')
+  })
+})

packages/delivery-electron/delivery.js

@@ -18,6 +18,11 @@ const delivery = (client, filestore, net, app) => {
       } else {
         const err = new Error(`Bad status code from API: ${response.statusCode}`)
         err.isRetryable = isRetryable(response.statusCode)
+        // do not retry oversized payloads regardless of status code
+        if (body.length > 10e5) {
+          client._logger.warn(`Discarding over-sized event (${(body.length / 10e5).toFixed(2)} MB) after failed delivery`)
+          err.isRetryable = false
+        }
         cb(err)
       }
     })

packages/delivery-electron/test/delivery.test-main.ts

@@ -207,6 +207,42 @@ describe('delivery: electron', () => {
     })
   })
 
+  it('does not attempt to re-send oversized payloads', done => {
+    // A 401 is considered retryable but this will be override by the payload size check
+    const { requests, server } = mockServer(401)
+    server.listen(err => {
+      expect(err).toBeUndefined()
+
+      const lotsOfEvents: any[] = []
+      while (JSON.stringify(lotsOfEvents).length < 10e5) {
+        lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] })
+      }
+      const payload = {
+        events: lotsOfEvents
+      } as unknown as EventDeliveryPayload
+
+      const config = {
+        apiKey: 'aaaaaaaa',
+        endpoints: { notify: `http://localhost:${(server.address() as AddressInfo).port}/notify/` },
+        redactedKeys: []
+      }
+
+      const logger = { error: jest.fn(), info: () => {}, warn: jest.fn() }
+      delivery(filestore, net, app)(makeClient(config, logger)).sendEvent(payload, (err: any) => {
+        expect(logger.error).toHaveBeenCalledWith(
+          expect.stringContaining('event failed to send…'),
+          expect.stringContaining('Bad status code from API: 401')
+        )
+        expect(logger.warn).toHaveBeenCalledWith('Discarding over-sized event (1.01 MB) after failed delivery')
+        expect(enqueueSpy).not.toHaveBeenCalled()
+        expect(err).toBeTruthy()
+        expect(requests.length).toBe(1)
+        server.close()
+        done()
+      })
+    })
+  })
+
   it('handles errors gracefully for sessions (ECONNREFUSED)', done => {
     const payload = {
       events: [{ errors: [{ errorClass: 'Error', errorMessage: 'foo is not a function' }] }]

packages/delivery-node/delivery.js

@@ -3,8 +3,13 @@ const request = require('./request')
 
 module.exports = (client) => ({
   sendEvent: (event, cb = () => {}) => {
+    const body = payload.event(event, client._config.redactedKeys)
+
     const _cb = err => {
       if (err) client._logger.error(`Event failed to send…\n${(err && err.stack) ? err.stack : err}`, err)
+      if (body.length > 10e5) {
+        client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`)
+      }
       cb(err)
     }
 
@@ -17,7 +22,7 @@ module.exports = (client) => ({
           'Bugsnag-Payload-Version': '4',
           'Bugsnag-Sent-At': (new Date()).toISOString()
         },
-        body: payload.event(event, client._config.redactedKeys),
+        body,
         agent: client._config.agent
       }, (err, body) => _cb(err))
     } catch (e) {

packages/delivery-node/test/delivery.test.ts

@@ -61,6 +61,38 @@ describe('delivery:node', () => {
     })
   })
 
+  it('logs failures and large payloads', done => {
+    const { server } = mockServer(400)
+    server.listen((err: Error) => {
+      expect(err).toBeUndefined()
+
+      const lotsOfEvents: any[] = []
+      while (JSON.stringify(lotsOfEvents).length < 10e5) {
+        lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] })
+      }
+      const payload = {
+        events: lotsOfEvents
+      } as unknown as EventDeliveryPayload
+
+      const config = {
+        apiKey: 'aaaaaaaa',
+        endpoints: { notify: `http://0.0.0.0:${(server.address() as AddressInfo).port}/notify/` },
+        redactedKeys: []
+      }
+
+      const logger = { error: jest.fn(), warn: jest.fn() }
+
+      delivery({ _logger: logger, _config: config } as unknown as Client).sendEvent(payload, (err) => {
+        expect(err).toStrictEqual(new Error('Bad statusCode from API: 400\nOK'))
+        expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('Event failed to send…'), expect.any(Error))
+        expect(logger.warn).toHaveBeenCalledWith('Event oversized (1.01 MB)')
+
+        server.close()
+        done()
+      })
+    })
+  })
+
   it('sends sessions successfully', done => {
     const { requests, server } = mockServer(202)
     server.listen((err: Error) => {

packages/delivery-x-domain-request/delivery.js

@@ -3,14 +3,24 @@ const payload = require('@bugsnag/core/lib/json-payload')
 module.exports = (client, win = window) => ({
   sendEvent: (event, cb = () => {}) => {
     const url = getApiUrl(client._config, 'notify', '4', win)
+    const body = payload.event(event, client._config.redactedKeys)
+
     const req = new win.XDomainRequest()
     req.onload = function () {
       cb(null)
     }
+    req.onerror = function () {
+      const err = new Error('Event failed to send')
+      client._logger.error('Event failed to send…', err)
+      if (body.length > 10e5) {
+        client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`)
+      }
+      cb(err)
+    }
     req.open('POST', url)
     setTimeout(() => {
       try {
-        req.send(payload.event(event, client._config.redactedKeys))
+        req.send(body)
       } catch (e) {
         client._logger.error(e)
         cb(e)

packages/delivery-x-domain-request/test/delivery.test.ts

@@ -72,6 +72,42 @@ describe('delivery:XDomainRequest', () => {
     })
   })
 
+  it('logs failures and large payloads', done => {
+    // mock XDomainRequest class
+    function XDomainRequest () {
+    }
+    XDomainRequest.prototype.open = function (method: string, url: string) {
+      this.method = method
+      this.url = url
+    }
+    XDomainRequest.prototype.send = function (method: string, url: string) {
+      this.onerror()
+    }
+    const window = { XDomainRequest, location: { protocol: 'https://' } } as unknown as Window
+
+    const lotsOfEvents: any[] = []
+    while (JSON.stringify(lotsOfEvents).length < 10e5) {
+      lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] })
+    }
+    const payload = {
+      events: lotsOfEvents
+    } as unknown as EventDeliveryPayload
+
+    const config = {
+      apiKey: 'aaaaaaaa',
+      endpoints: { notify: '/echo/', sessions: '/sessions/' },
+      redactedKeys: []
+    }
+    const logger = { error: jest.fn(), warn: jest.fn() }
+    delivery({ _logger: logger, _config: config } as unknown as Client, window).sendEvent(payload, (err) => {
+      const expectedError = new Error('Event failed to send')
+      expect(err).toStrictEqual(expectedError)
+      expect(logger.error).toHaveBeenCalledWith('Event failed to send…', expectedError)
+      expect(logger.warn).toHaveBeenCalledWith('Event oversized (1.01 MB)')
+      done()
+    })
+  })
+
   it('sends sessions successfully', done => {
     const requests: XDomainRequest[] = []
 

packages/delivery-xml-http-request/delivery.js

@@ -5,15 +5,26 @@ module.exports = (client, win = window) => ({
     try {
       const url = client._config.endpoints.notify
       const req = new win.XMLHttpRequest()
+      const body = payload.event(event, client._config.redactedKeys)
+
       req.onreadystatechange = function () {
-        if (req.readyState === win.XMLHttpRequest.DONE) cb(null)
+        if (req.readyState === win.XMLHttpRequest.DONE) {
+          const status = req.status
+          if (status === 0 || status >= 400) {
+            client._logger.error('Event failed to send…')
+            if (body.length > 10e5) {
+              client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`)
+            }
+          }
+          cb(null)
+        }
       }
       req.open('POST', url)
       req.setRequestHeader('Content-Type', 'application/json')
       req.setRequestHeader('Bugsnag-Api-Key', event.apiKey || client._config.apiKey)
       req.setRequestHeader('Bugsnag-Payload-Version', '4')
       req.setRequestHeader('Bugsnag-Sent-At', (new Date()).toISOString())
-      req.send(payload.event(event, client._config.redactedKeys))
+      req.send(body)
     } catch (e) {
       client._logger.error(e)
     }