From e344fd297dbc857f989d9ce0342c754b16273b96 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 29 Sep 2022 14:57:46 +0100 Subject: [PATCH 1/4] explicitly mark failed payloads >1MB as not retryable --- packages/delivery-expo/delivery.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/delivery-expo/delivery.js b/packages/delivery-expo/delivery.js index 2b87879e..2f12b229 100644 --- a/packages/delivery-expo/delivery.js +++ b/packages/delivery-expo/delivery.js @@ -61,7 +61,14 @@ module.exports = (client, fetch = global.fetch) => { } client._logger.info(`Sending event ${event.events[0].errors[0].errorClass}: ${event.events[0].errors[0].errorMessage}`) send(url, opts, err => { - if (err) return onerror(err, { url, opts }, 'event', cb) + if (err) { + // do not retry oversized payloads regardless of status code + if (body.length > 10e5) { + client._logger.warn(`Discarding over-sized event (${body.length / 10e5}) after failed delivery`) + err.isRetryable = false + } + return onerror(err, { url, opts }, 'event', cb) + } cb(null) }) } catch (e) { From 3be1dc1600f821fa2a66039471294cea543ee74f Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 29 Sep 2022 16:57:05 +0100 Subject: [PATCH 2/4] explicitly mark failed payloads >1MB as not retryable --- packages/delivery-expo/test/delivery.test.js | 34 ++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/delivery-expo/test/delivery.test.js b/packages/delivery-expo/test/delivery.test.js index a3c15e66..4826c82d 100644 --- a/packages/delivery-expo/test/delivery.test.js +++ b/packages/delivery-expo/test/delivery.test.js @@ -202,6 +202,40 @@ describe('delivery: expo', () => { }) }) + 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 = [] + while (JSON.stringify(lotsOfEvents).length < 10e5) { + lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] }) + } + const payload = { + events: lotsOfEvents + } + + const config = { + apiKey: 'aaaaaaaa', + endpoints: { notify: `http://0.0.0.0:${server.address().port}/notify/` }, + redactedKeys: [] + } + + let didLog = false + const log = () => { didLog = true } + + delivery({ _config: config, _logger: { error: log, info: () => {} } }, fetch).sendEvent(payload, (err) => { + expect(didLog).toBe(true) + 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' }] }] From 898c1b12da244cd8bca70e5819ce43af36feb791 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 29 Sep 2022 17:20:50 +0100 Subject: [PATCH 3/4] explicitly mark failed payloads >1MB as not retryable --- packages/delivery-expo/delivery.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/delivery-expo/delivery.js b/packages/delivery-expo/delivery.js index 2f12b229..a888b88d 100644 --- a/packages/delivery-expo/delivery.js +++ b/packages/delivery-expo/delivery.js @@ -64,7 +64,7 @@ module.exports = (client, fetch = global.fetch) => { if (err) { // do not retry oversized payloads regardless of status code if (body.length > 10e5) { - client._logger.warn(`Discarding over-sized event (${body.length / 10e5}) after failed delivery`) + client._logger.warn(`Discarding over-sized event (${body.length / 10e5} MB) after failed delivery`) err.isRetryable = false } return onerror(err, { url, opts }, 'event', cb) From 27cc9eaf5337971f7cb44251830703ae342c1351 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Wed, 22 Feb 2023 16:22:07 +0000 Subject: [PATCH 4/4] fix expo oversized delivery payload test --- packages/delivery-expo/test/delivery.test.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/delivery-expo/test/delivery.test.js b/packages/delivery-expo/test/delivery.test.js index 4826c82d..b13ec94f 100644 --- a/packages/delivery-expo/test/delivery.test.js +++ b/packages/delivery-expo/test/delivery.test.js @@ -203,7 +203,7 @@ describe('delivery: expo', () => { }) 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 + // A 401 is considered retryable but this will be overridden by the payload size check const { requests, server } = mockServer(401) server.listen(err => { expect(err).toBeUndefined() @@ -222,14 +222,17 @@ describe('delivery: expo', () => { redactedKeys: [] } - let didLog = false - const log = () => { didLog = true } + const logger = { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn() + } - delivery({ _config: config, _logger: { error: log, info: () => {} } }, fetch).sendEvent(payload, (err) => { - expect(didLog).toBe(true) + delivery({ _config: config, _logger: logger }, fetch).sendEvent(payload, (err) => { + expect(logger.warn).toHaveBeenCalledWith('Discarding over-sized event (1.014603 MB) after failed delivery') expect(enqueueSpy).not.toHaveBeenCalled() expect(err).toBeTruthy() - expect(requests.length).toBe(1) + expect(requests.length).toBe(0) server.close() done() })