From 1387119141b1525c0c3e69a7cb9b221eb8ba1e71 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 24 Feb 2021 12:18:44 +0000 Subject: [PATCH 1/4] Add '_depth' to internal Client type --- packages/core/client.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/client.d.ts b/packages/core/client.d.ts index 8b251eb1c8..72f30ae42e 100644 --- a/packages/core/client.d.ts +++ b/packages/core/client.d.ts @@ -41,6 +41,7 @@ interface Delivery { export default class ClientWithInternals extends Client { public constructor(opts: T, schema?: {[key: string]: any}, internalPlugins?: Plugin[], notifier?: Notifier) _config: T + _depth: number _logger: LoggerConfig _breadcrumbs: Breadcrumb[]; _delivery: Delivery From 5162bb8ef88f7862a8e6abcba59fdbeb3026c980 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 24 Feb 2021 11:32:33 +0000 Subject: [PATCH 2/4] Initial package scaffolding --- jest.config.js | 1 + packages/in-flight/LICENSE.txt | 19 ++++++++++++ packages/in-flight/README.md | 7 +++++ packages/in-flight/package-lock.json | 13 ++++++++ packages/in-flight/package.json | 30 +++++++++++++++++++ packages/in-flight/src/in-flight.js | 0 packages/in-flight/test/in-flight.test.ts | 7 +++++ .../in-flight/types/bugsnag-in-flight.d.ts | 0 tsconfig.json | 1 + 9 files changed, 78 insertions(+) create mode 100644 packages/in-flight/LICENSE.txt create mode 100644 packages/in-flight/README.md create mode 100644 packages/in-flight/package-lock.json create mode 100644 packages/in-flight/package.json create mode 100644 packages/in-flight/src/in-flight.js create mode 100644 packages/in-flight/test/in-flight.test.ts create mode 100644 packages/in-flight/types/bugsnag-in-flight.d.ts diff --git a/jest.config.js b/jest.config.js index 7611ae701d..527502ae91 100644 --- a/jest.config.js +++ b/jest.config.js @@ -78,6 +78,7 @@ module.exports = { ]), project('node plugins', [ 'delivery-node', + 'in-flight', 'plugin-express', 'plugin-koa', 'plugin-restify', diff --git a/packages/in-flight/LICENSE.txt b/packages/in-flight/LICENSE.txt new file mode 100644 index 0000000000..ddc0631e24 --- /dev/null +++ b/packages/in-flight/LICENSE.txt @@ -0,0 +1,19 @@ +Copyright (c) Bugsnag, https://www.bugsnag.com/ + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the "Software"), +to deal in the Software without restriction, including without limitation +the rights to use, copy, modify, merge, publish, distribute, sublicense, +and/or sell copies of the Software, and to permit persons to whom the Software +is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/packages/in-flight/README.md b/packages/in-flight/README.md new file mode 100644 index 0000000000..388679e7a3 --- /dev/null +++ b/packages/in-flight/README.md @@ -0,0 +1,7 @@ +# @bugsnag/in-flight + +Internal [@bugsnag/js](https://github.com/bugsnag/bugsnag-js) package to keep track of in-flight requests + +## License + +This package is free software released under the MIT License. See [LICENSE.txt](./LICENSE.txt) for details. diff --git a/packages/in-flight/package-lock.json b/packages/in-flight/package-lock.json new file mode 100644 index 0000000000..d0881cfb52 --- /dev/null +++ b/packages/in-flight/package-lock.json @@ -0,0 +1,13 @@ +{ + "name": "@bugsnag/in-flight", + "version": "7.7.0", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "@bugsnag/cuid": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@bugsnag/cuid/-/cuid-3.0.0.tgz", + "integrity": "sha512-LOt8aaBI+KvOQGneBtpuCz3YqzyEAehd1f3nC5yr9TIYW1+IzYKa2xWS4EiMz5pPOnRPHkyyS5t/wmSmN51Gjg==" + } + } +} diff --git a/packages/in-flight/package.json b/packages/in-flight/package.json new file mode 100644 index 0000000000..4e08fd75f2 --- /dev/null +++ b/packages/in-flight/package.json @@ -0,0 +1,30 @@ +{ + "name": "@bugsnag/in-flight", + "version": "7.7.0", + "main": "src/in-flight.js", + "types": "types/bugsnag-in-flight.d.ts", + "description": "Internal package to keep track of in-flight requests to Bugsnag", + "homepage": "https://www.bugsnag.com/", + "repository": { + "type": "git", + "url": "git@github.com:bugsnag/bugsnag-js.git" + }, + "publishConfig": { + "access": "public" + }, + "files": [ + "src", + "types" + ], + "author": "Bugsnag", + "license": "MIT", + "dependencies": { + "@bugsnag/cuid": "^3.0.0" + }, + "devDependencies": { + "@bugsnag/core": "^7.7.0" + }, + "peerDependencies": { + "@bugsnag/core": "^7.0.0" + } +} diff --git a/packages/in-flight/src/in-flight.js b/packages/in-flight/src/in-flight.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/in-flight/test/in-flight.test.ts b/packages/in-flight/test/in-flight.test.ts new file mode 100644 index 0000000000..b483b2b846 --- /dev/null +++ b/packages/in-flight/test/in-flight.test.ts @@ -0,0 +1,7 @@ +describe('@bugsnag/in-flight', () => { + it.todo('tracks in-flight events') + it.todo('tracks in-flight sessions') + it.todo('tracks all in-flight requests') + it.todo('can flush successfully') + it.todo('will timeout if flush takes too long') +}) diff --git a/packages/in-flight/types/bugsnag-in-flight.d.ts b/packages/in-flight/types/bugsnag-in-flight.d.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tsconfig.json b/tsconfig.json index c5eb792567..eb5dc389de 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -70,6 +70,7 @@ "packages/delivery-react-native", "packages/delivery-x-domain-request", "packages/delivery-xml-http-request", + "packages/in-flight", "packages/plugin-app-duration", "packages/plugin-browser-context", "packages/plugin-browser-device", From ca27c26999657c51829a6d24a4c555107af63baf Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 24 Feb 2021 12:19:04 +0000 Subject: [PATCH 3/4] Allow waiting for in-flight requests to complete This monkey-patches the given client to keep track of the number of in-flight requests and allows waiting until all requests have finished (with a timeout) --- packages/in-flight/src/in-flight.js | 67 ++++++ packages/in-flight/test/in-flight.test.ts | 222 +++++++++++++++++- .../in-flight/types/bugsnag-in-flight.d.ts | 8 + 3 files changed, 292 insertions(+), 5 deletions(-) diff --git a/packages/in-flight/src/in-flight.js b/packages/in-flight/src/in-flight.js index e69de29bb2..89fc7bb072 100644 --- a/packages/in-flight/src/in-flight.js +++ b/packages/in-flight/src/in-flight.js @@ -0,0 +1,67 @@ +const cuid = require('@bugsnag/cuid') + +const FLUSH_POLL_INTERVAL_MS = 50 +const inFlightRequests = new Map() + +const noop = () => {} + +module.exports = { + trackInFlight (client) { + const originalNotify = client._notify + + client._notify = function (event, onError, callback = noop) { + const id = cuid() + inFlightRequests.set(id, true) + + const _callback = function () { + inFlightRequests.delete(id) + callback.apply(null, arguments) + } + + client._depth += 1 + + try { + originalNotify.call(client, event, onError, _callback) + } finally { + client._depth -= 1 + } + } + + const delivery = client._delivery + const originalSendSession = delivery.sendSession + + delivery.sendSession = function (session, callback = noop) { + const id = cuid() + inFlightRequests.set(id, true) + + const _callback = function () { + inFlightRequests.delete(id) + callback.apply(null, arguments) + } + + originalSendSession.call(delivery, session, _callback) + } + }, + + flush (timeoutMs) { + return new Promise(function (resolve, reject) { + const timeout = setTimeout( + () => { reject(new Error(`flush timed out after ${timeoutMs}ms`)) }, + timeoutMs + ) + + const resolveIfNoRequests = function () { + if (inFlightRequests.size === 0) { + clearTimeout(timeout) + resolve() + + return + } + + setTimeout(resolveIfNoRequests, FLUSH_POLL_INTERVAL_MS) + } + + resolveIfNoRequests() + }) + } +} diff --git a/packages/in-flight/test/in-flight.test.ts b/packages/in-flight/test/in-flight.test.ts index b483b2b846..76abb4cb7b 100644 --- a/packages/in-flight/test/in-flight.test.ts +++ b/packages/in-flight/test/in-flight.test.ts @@ -1,7 +1,219 @@ +import Client, { EventDeliveryPayload, SessionDeliveryPayload } from '@bugsnag/core/client' + +// The in-flight package has module level state which can leak between tests +// We can avoid this using jest's 'isolateModules' but need to type the +// 'bugsnagInFlight' variable for this test to compile +import BugsnagInFlightJustForTypescript from '../types/bugsnag-in-flight' + +let bugsnagInFlight: BugsnagInFlightJustForTypescript +jest.isolateModules(() => { bugsnagInFlight = require('../src/in-flight') }) + describe('@bugsnag/in-flight', () => { - it.todo('tracks in-flight events') - it.todo('tracks in-flight sessions') - it.todo('tracks all in-flight requests') - it.todo('can flush successfully') - it.todo('will timeout if flush takes too long') + it('tracks in-flight events', () => { + const client = new Client({ apiKey: 'AN_API_KEY' }) + const payloads: EventDeliveryPayload[] = [] + const sendSession = jest.fn() + + client._setDelivery(() => ({ + sendEvent: (payload, cb) => { + expect(client._depth).toBe(2) + payloads.push(payload) + cb() + }, + sendSession + })) + + bugsnagInFlight.trackInFlight(client) + + expect(payloads.length).toBe(0) + + const onError = jest.fn() + const callback = jest.fn() + + expect(client._depth).toBe(1) + + client.notify(new Error('xyz'), onError, callback) + + expect(client._depth).toBe(1) + expect(onError).toHaveBeenCalledTimes(1) + expect(callback).toHaveBeenCalledTimes(1) + expect(payloads.length).toBe(1) + expect(sendSession).not.toHaveBeenCalled() + }) + + it('tracks in-flight sessions', () => { + const client = new Client({ apiKey: 'AN_API_KEY' }) + const payloads: SessionDeliveryPayload[] = [] + const sendEvent = jest.fn() + const callback = jest.fn() + + client._sessionDelegate = { + startSession: jest.fn(function (client, session) { + client._delivery.sendSession(session, callback) + }), + pauseSession: jest.fn(), + resumeSession: jest.fn() + } + + client._setDelivery(() => ({ + sendEvent, + sendSession: (payload, cb) => { + payloads.push(payload) + cb() + } + })) + + bugsnagInFlight.trackInFlight(client) + + expect(payloads.length).toBe(0) + expect(callback).not.toHaveBeenCalled() + expect(client._sessionDelegate.startSession).not.toHaveBeenCalled() + expect(client._sessionDelegate.pauseSession).not.toHaveBeenCalled() + expect(client._sessionDelegate.resumeSession).not.toHaveBeenCalled() + + client.startSession() + + expect(payloads.length).toBe(1) + expect(callback).toHaveBeenCalledTimes(1) + expect(client._sessionDelegate.startSession).toHaveBeenCalledTimes(1) + expect(client._sessionDelegate.pauseSession).not.toHaveBeenCalled() + expect(client._sessionDelegate.resumeSession).not.toHaveBeenCalled() + }) + + it('tracks all in-flight requests', () => { + const client = new Client({ apiKey: 'AN_API_KEY' }) + const eventPayloads: EventDeliveryPayload[] = [] + const sessionPayloads: SessionDeliveryPayload[] = [] + const sessionCallback = jest.fn() + + client._sessionDelegate = { + startSession: jest.fn(function (client, session) { + client._delivery.sendSession(session, sessionCallback) + }), + pauseSession: jest.fn(), + resumeSession: jest.fn() + } + + client._setDelivery(() => ({ + sendEvent: (payload, cb) => { + expect(client._depth).toBe(2) + eventPayloads.push(payload) + cb() + }, + sendSession: (payload, cb) => { + sessionPayloads.push(payload) + cb() + } + })) + + bugsnagInFlight.trackInFlight(client) + + expect(eventPayloads.length).toBe(0) + expect(sessionPayloads.length).toBe(0) + + const onError = jest.fn() + const notifyCallback = jest.fn() + + expect(client._depth).toBe(1) + + client.notify(new Error('xyz'), onError, notifyCallback) + client.startSession() + + expect(client._depth).toBe(1) + expect(onError).toHaveBeenCalledTimes(1) + expect(notifyCallback).toHaveBeenCalledTimes(1) + expect(sessionCallback).toHaveBeenCalledTimes(1) + expect(eventPayloads.length).toBe(1) + expect(sessionPayloads.length).toBe(1) + }) + + it('can flush successfully', async () => { + const client = new Client({ apiKey: 'AN_API_KEY' }) + const eventPayloads: EventDeliveryPayload[] = [] + const sessionPayloads: SessionDeliveryPayload[] = [] + + client._sessionDelegate = { + startSession (client, session) { + client._delivery.sendSession(session, () => {}) + }, + pauseSession: () => {}, + resumeSession: () => {} + } + + client._setDelivery(() => ({ + sendEvent (payload, cb) { + setTimeout(function () { + eventPayloads.push(payload) + cb() + }, 100) + }, + sendSession (payload, cb) { + setTimeout(function () { + sessionPayloads.push(payload) + cb() + }, 100) + } + })) + + bugsnagInFlight.trackInFlight(client) + + client.notify(new Error('xyz')) + client.startSession() + + expect(eventPayloads.length).toBe(0) + expect(sessionPayloads.length).toBe(0) + + await bugsnagInFlight.flush(1000) + + expect(eventPayloads.length).toBe(1) + expect(sessionPayloads.length).toBe(1) + }) + + it('will timeout if flush takes too long', async () => { + const client = new Client({ apiKey: 'AN_API_KEY' }) + const eventPayloads: EventDeliveryPayload[] = [] + const sessionPayloads: SessionDeliveryPayload[] = [] + + client._sessionDelegate = { + startSession: (client, session) => { + client._delivery.sendSession(session, () => {}) + }, + pauseSession: () => {}, + resumeSession: () => {} + } + + client._setDelivery(() => ({ + sendEvent (payload, cb) { + setTimeout(() => { + eventPayloads.push(payload) + cb() + }, 250) + }, + sendSession (payload, cb) { + setTimeout(() => { + sessionPayloads.push(payload) + cb() + }, 250) + } + })) + + bugsnagInFlight.trackInFlight(client) + + client.notify(new Error('xyz')) + client.startSession() + + expect(eventPayloads.length).toBe(0) + expect(sessionPayloads.length).toBe(0) + + const expected = new Error('flush timed out after 10ms') + await expect(() => bugsnagInFlight.flush(10)).rejects.toThrow(expected) + + expect(eventPayloads.length).toBe(0) + expect(sessionPayloads.length).toBe(0) + + await bugsnagInFlight.flush(1000) + + expect(eventPayloads.length).toBe(1) + expect(sessionPayloads.length).toBe(1) + }) }) diff --git a/packages/in-flight/types/bugsnag-in-flight.d.ts b/packages/in-flight/types/bugsnag-in-flight.d.ts index e69de29bb2..c2dd1a86ea 100644 --- a/packages/in-flight/types/bugsnag-in-flight.d.ts +++ b/packages/in-flight/types/bugsnag-in-flight.d.ts @@ -0,0 +1,8 @@ +import { Client } from '@bugsnag/core' + +interface BugsnagInFlight { + trackInFlight (client: Client): void + flush (timeoutMs: number): Promise +} + +export default BugsnagInFlight From 99d3e00c267e83f22af87f3d56e0faa99f7359d6 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 24 Feb 2021 17:39:26 +0000 Subject: [PATCH 4/4] Avoid resolving after ject has been called This should never cause issues as the ES spec guarantees promises that are resolved/rejected can't be resolved or rejected again, however this is such a small change and is a bit more complete/safe --- packages/in-flight/src/in-flight.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/in-flight/src/in-flight.js b/packages/in-flight/src/in-flight.js index 89fc7bb072..c85b287fa7 100644 --- a/packages/in-flight/src/in-flight.js +++ b/packages/in-flight/src/in-flight.js @@ -45,20 +45,25 @@ module.exports = { flush (timeoutMs) { return new Promise(function (resolve, reject) { - const timeout = setTimeout( - () => { reject(new Error(`flush timed out after ${timeoutMs}ms`)) }, + let resolveTimeout + const rejectTimeout = setTimeout( + () => { + if (resolveTimeout) clearTimeout(resolveTimeout) + + reject(new Error(`flush timed out after ${timeoutMs}ms`)) + }, timeoutMs ) const resolveIfNoRequests = function () { if (inFlightRequests.size === 0) { - clearTimeout(timeout) + clearTimeout(rejectTimeout) resolve() return } - setTimeout(resolveIfNoRequests, FLUSH_POLL_INTERVAL_MS) + resolveTimeout = setTimeout(resolveIfNoRequests, FLUSH_POLL_INTERVAL_MS) } resolveIfNoRequests()