diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3e507bd5..73e2f11a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -63,6 +63,7 @@ jobs: node-version-file: 'package.json' - run: yarn install - name: Run Unit Tests + timeout-minutes: 10 run: yarn test e2e: diff --git a/package.json b/package.json index 4b37f329..68ab10c7 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "pretest": "yarn build", "test": "vitest run --root=./test/unit", "pree2e": "rimraf --glob test/e2e/dist/**/node_modules/@sentry/** test/e2e/dist/**/yarn.lock test/e2e/dist/**/package-lock.json && node scripts/clean-cache.js && yarn build && npm pack", - "e2e": "xvfb-maybe vitest run --root=./test/e2e" + "e2e": "xvfb-maybe vitest run --root=./test/e2e --silent=false --disable-console-intercept" }, "dependencies": { "@sentry/browser": "8.0.0-alpha.9", diff --git a/src/main/transports/electron-offline-net.ts b/src/main/transports/electron-offline-net.ts index 8beafc10..e812d2da 100644 --- a/src/main/transports/electron-offline-net.ts +++ b/src/main/transports/electron-offline-net.ts @@ -1,153 +1,23 @@ -import { createTransport } from '@sentry/core'; -import { Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; -import { logger } from '@sentry/utils'; -import { net } from 'electron'; -import { join } from 'path'; +import { makeOfflineTransport, OfflineTransportOptions } from '@sentry/core'; +import { BaseTransportOptions, Transport } from '@sentry/types'; -import { getSentryCachePath } from '../electron-normalize'; -import { createElectronNetRequestExecutor, ElectronNetTransportOptions } from './electron-net'; -import { PersistedRequestQueue } from './queue'; +import { ElectronNetTransportOptions, makeElectronTransport } from './electron-net'; +import { createOfflineStore, OfflineStoreOptions } from './offline-store'; -type BeforeSendResponse = 'send' | 'queue' | 'drop'; - -export interface ElectronOfflineTransportOptions extends ElectronNetTransportOptions { - /** - * The maximum number of days to keep an event in the queue. - */ - maxQueueAgeDays?: number; - - /** - * The maximum number of events to keep in the queue. - */ - maxQueueCount?: number; - - /** - * Called every time the number of requests in the queue changes. - */ - queuedLengthChanged?: (length: number) => void; - - /** - * Called before attempting to send an event to Sentry. - * - * Return 'send' to attempt to send the event. - * Return 'queue' to queue and persist the event for sending later. - * Return 'drop' to drop the event. - */ - beforeSend?: (request: TransportRequest) => BeforeSendResponse | Promise; -} - -const START_DELAY = 5_000; -const MAX_DELAY = 2_000_000_000; - -/** Returns true is there's a chance we're online */ -function maybeOnline(): boolean { - return !('online' in net) || net.online === true; -} - -function defaultBeforeSend(_: TransportRequest): BeforeSendResponse { - return maybeOnline() ? 'send' : 'queue'; -} - -function isRateLimited(result: TransportMakeRequestResponse): boolean { - return !!(result.headers && 'x-sentry-rate-limits' in result.headers); -} +export type ElectronOfflineTransportOptions = ElectronNetTransportOptions & + OfflineTransportOptions & + Partial; /** * Creates a Transport that uses Electrons net module to send events to Sentry. When they fail to send they are * persisted to disk and sent later */ -export function makeElectronOfflineTransport(options: ElectronOfflineTransportOptions): Transport { - const netMakeRequest = createElectronNetRequestExecutor(options.url, options.headers || {}); - const queue: PersistedRequestQueue = new PersistedRequestQueue( - join(getSentryCachePath(), 'queue'), - options.maxQueueAgeDays, - options.maxQueueCount, - ); - - const beforeSend = options.beforeSend || defaultBeforeSend; - - let retryDelay: number = START_DELAY; - let lastQueueLength = -1; - - function queueLengthChanged(queuedEvents: number): void { - if (options.queuedLengthChanged && queuedEvents !== lastQueueLength) { - lastQueueLength = queuedEvents; - options.queuedLengthChanged(queuedEvents); - } - } - - function flushQueue(): void { - queue - .pop() - .then((found) => { - if (found) { - // We have pendingCount plus found.request - queueLengthChanged(found.pendingCount + 1); - logger.log('Found a request in the queue'); - makeRequest(found.request).catch((e) => logger.error(e)); - } else { - queueLengthChanged(0); - } - }) - .catch((e) => logger.error(e)); - } - - async function queueRequest(request: TransportRequest): Promise { - logger.log('Queuing request'); - queueLengthChanged(await queue.add(request)); - - setTimeout(() => { - flushQueue(); - }, retryDelay); - - retryDelay *= 3; - - // If the delay is bigger than 2^31 (max signed 32-bit int), setTimeout throws - // an error and falls back to 1 which can cause a huge number of requests. - if (retryDelay > MAX_DELAY) { - retryDelay = MAX_DELAY; - } - - return {}; - } - - async function makeRequest(request: TransportRequest): Promise { - let action = beforeSend(request); - - if (action instanceof Promise) { - action = await action; - } - - if (action === 'send') { - try { - const result = await netMakeRequest(request); - - if (!isRateLimited(result)) { - logger.log('Successfully sent'); - // Reset the retry delay - retryDelay = START_DELAY; - // We were successful so check the queue - flushQueue(); - return result; - } else { - logger.log('Rate limited', result.headers); - } - } catch (error) { - logger.log('Error sending:', error); - } - - action = 'queue'; - } - - if (action == 'queue') { - return queueRequest(request); - } - - logger.log('Dropping request'); - return {}; - } - - flushQueue(); - - return createTransport(options, makeRequest); -} +export const makeElectronOfflineTransport = ( + options: T & ElectronOfflineTransportOptions, +): Transport => { + return makeOfflineTransport(makeElectronTransport)({ + flushAtStartup: true, + ...options, + createStore: createOfflineStore, + }); +}; diff --git a/src/main/transports/offline-store.ts b/src/main/transports/offline-store.ts new file mode 100644 index 00000000..ffb5b0b3 --- /dev/null +++ b/src/main/transports/offline-store.ts @@ -0,0 +1,124 @@ +import { OfflineStore } from '@sentry/core'; +import { Envelope } from '@sentry/types'; +import { logger, parseEnvelope, serializeEnvelope, uuid4 } from '@sentry/utils'; +import { promises as fs } from 'fs'; +import { join } from 'path'; + +import { getSentryCachePath } from '../electron-normalize'; +import { Store } from '../store'; + +/** Internal type used to expose the envelope date without having to read it into memory */ +interface PersistedRequest { + id: string; + date: Date; +} + +export interface OfflineStoreOptions { + /** + * Path to the offline queue directory. + */ + queuePath: string; + /** + * Maximum number of days to store requests. + */ + maxAgeDays: number; + /** + * Maximum number of requests to store. + */ + maxQueueSize: number; +} + +const MILLISECONDS_PER_DAY = 86_400_000; + +function isOutdated(request: PersistedRequest, maxAgeDays: number): boolean { + const cutOff = Date.now() - MILLISECONDS_PER_DAY * maxAgeDays; + return request.date.getTime() < cutOff; +} + +function getSentAtFromEnvelope(envelope: Envelope): Date | undefined { + const header = envelope[0]; + if (typeof header.sent_at === 'string') { + return new Date(header.sent_at); + } + return undefined; +} + +/** + * Creates a new offline store. + */ +export function createOfflineStore(userOptions: Partial): OfflineStore { + function log(...args: unknown[]): void { + logger.log(`[Offline Store]:`, ...args); + } + + const options: OfflineStoreOptions = { + maxAgeDays: userOptions.maxAgeDays || 30, + maxQueueSize: userOptions.maxQueueSize || 30, + queuePath: userOptions.queuePath || join(getSentryCachePath(), 'queue'), + }; + const queue = new Store(options.queuePath, 'queue-v2', []); + + function removeBody(id: string): void { + fs.unlink(join(options.queuePath, id)).catch(() => { + // ignore + }); + } + + function removeStaleRequests(queue: PersistedRequest[]): void { + while (queue[0] && isOutdated(queue[0], options.maxAgeDays)) { + const removed = queue.shift() as PersistedRequest; + log('Removing stale envelope', removed); + removeBody(removed.id); + } + } + + return { + insert: async (env) => { + log('Adding envelope to offline storage'); + + const id = uuid4(); + + try { + const data = serializeEnvelope(env); + await fs.mkdir(options.queuePath, { recursive: true }); + await fs.writeFile(join(options.queuePath, id), data); + } catch (e) { + log('Failed to save', e); + } + + await queue.update((queue) => { + removeStaleRequests(queue); + + if (queue.length >= options.maxQueueSize) { + removeBody(id); + return queue; + } + + queue.push({ id, date: getSentAtFromEnvelope(env) || new Date() }); + + return queue; + }); + }, + pop: async () => { + log('Popping envelope from offline storage'); + let request: PersistedRequest | undefined; + await queue.update((queue) => { + removeStaleRequests(queue); + request = queue.shift(); + return queue; + }); + + if (request) { + try { + const data = await fs.readFile(join(options.queuePath, request.id)); + removeBody(request.id); + return parseEnvelope(data); + } catch (e) { + log('Failed to read', e); + } + } + + return undefined; + }, + }; +} diff --git a/src/main/transports/queue.ts b/src/main/transports/queue.ts deleted file mode 100644 index bd22e544..00000000 --- a/src/main/transports/queue.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { TransportRequest } from '@sentry/types'; -import { logger, uuid4 } from '@sentry/utils'; -import { promises as fs } from 'fs'; -import { join } from 'path'; - -import { BufferedWriteStore } from '../store'; - -const MILLISECONDS_PER_DAY = 86_400_000; - -interface PersistedRequest { - bodyPath: string; - date: Date; - // Envelopes were persisted in a different format in v3 - // If this property exists, we discard this event - type?: unknown; -} - -interface PopResult { - request: QueuedTransportRequest; - pendingCount: number; -} - -export interface QueuedTransportRequest extends TransportRequest { - date?: Date; -} - -/** A request queue that is persisted to disk to survive app restarts */ -export class PersistedRequestQueue { - private readonly _queue: BufferedWriteStore; - - public constructor( - private readonly _queuePath: string, - private readonly _maxAgeDays: number = 30, - private readonly _maxCount: number = 30, - ) { - this._queue = new BufferedWriteStore(this._queuePath, 'queue', []); - } - - /** Adds a request to the queue */ - public async add(request: QueuedTransportRequest): Promise { - const bodyPath = uuid4(); - let queuedEvents = 0; - - await this._queue.update((queue) => { - queue.push({ - bodyPath, - date: request.date || new Date(), - }); - - while (queue.length > this._maxCount) { - const removed = queue.shift(); - if (removed) { - this._removeBody(removed.bodyPath); - } - } - - queuedEvents = queue.length; - return queue; - }); - - try { - await fs.writeFile(join(this._queuePath, bodyPath), request.body); - } catch (_) { - // - } - - return queuedEvents; - } - - /** Pops the oldest event from the queue */ - public async pop(): Promise { - let found: PersistedRequest | undefined; - let pendingCount = 0; - const cutOff = Date.now() - MILLISECONDS_PER_DAY * this._maxAgeDays; - - await this._queue.update((queue) => { - while ((found = queue.shift())) { - // We drop events created in v3 of the SDK or before the cut-off - if ('type' in found || found.date.getTime() < cutOff) { - // we're dropping this event so delete the body - this._removeBody(found.bodyPath); - found = undefined; - } else { - pendingCount = queue.length; - break; - } - } - return queue; - }); - - if (found) { - try { - const body = await fs.readFile(join(this._queuePath, found.bodyPath)); - this._removeBody(found.bodyPath); - - return { - request: { - body, - date: found.date || new Date(), - }, - pendingCount, - }; - } catch (e) { - logger.warn('Filed to read queued request body', e); - } - } - - return undefined; - } - - /** Removes the body of the request */ - private _removeBody(bodyPath: string): void { - fs.unlink(join(this._queuePath, bodyPath)).catch(() => { - // ignore - }); - } -} diff --git a/test/e2e/test-apps/offline/native-crash/recipe.yml b/test/e2e/test-apps/offline/native-crash/recipe.yml index eba165b9..fb56b6ab 100644 --- a/test/e2e/test-apps/offline/native-crash/recipe.yml +++ b/test/e2e/test-apps/offline/native-crash/recipe.yml @@ -1,4 +1,5 @@ -description: Native Crash Rate Limit +description: Native Crash Send Failure category: Offline command: yarn +condition: "!(platform === 'win32' && version.major < 24)" runTwice: true diff --git a/test/e2e/test-apps/offline/native-crash/src/main.js b/test/e2e/test-apps/offline/native-crash/src/main.js index 83eb31cb..bbb88046 100644 --- a/test/e2e/test-apps/offline/native-crash/src/main.js +++ b/test/e2e/test-apps/offline/native-crash/src/main.js @@ -4,9 +4,10 @@ const { app, BrowserWindow } = require('electron'); const { init } = require('@sentry/electron/main'); init({ - dsn: process.env.APP_FIRST_RUN ? '__RATE_LIMIT_DSN__' : '__DSN__', + dsn: process.env.APP_FIRST_RUN ? '__INCORRECT_DSN__' : '__DSN__', debug: true, autoSessionTracking: false, + transportOptions: { flushAtStartup: !process.env.APP_FIRST_RUN }, onFatalError: () => {}, }); diff --git a/test/e2e/test-apps/offline/renderer-error/recipe.yml b/test/e2e/test-apps/offline/renderer-error/recipe.yml index 622cf4a8..4933ed51 100644 --- a/test/e2e/test-apps/offline/renderer-error/recipe.yml +++ b/test/e2e/test-apps/offline/renderer-error/recipe.yml @@ -1,4 +1,4 @@ -description: JavaScript Error Rate Limit +description: JavaScript Error Send Failure category: Offline command: yarn runTwice: true diff --git a/test/e2e/test-apps/offline/renderer-error/src/main.js b/test/e2e/test-apps/offline/renderer-error/src/main.js index 83eb31cb..bbb88046 100644 --- a/test/e2e/test-apps/offline/renderer-error/src/main.js +++ b/test/e2e/test-apps/offline/renderer-error/src/main.js @@ -4,9 +4,10 @@ const { app, BrowserWindow } = require('electron'); const { init } = require('@sentry/electron/main'); init({ - dsn: process.env.APP_FIRST_RUN ? '__RATE_LIMIT_DSN__' : '__DSN__', + dsn: process.env.APP_FIRST_RUN ? '__INCORRECT_DSN__' : '__DSN__', debug: true, autoSessionTracking: false, + transportOptions: { flushAtStartup: !process.env.APP_FIRST_RUN }, onFatalError: () => {}, }); diff --git a/test/e2e/vitest.config.mts b/test/e2e/vitest.config.mts index eb5f7f80..cda74553 100644 --- a/test/e2e/vitest.config.mts +++ b/test/e2e/vitest.config.mts @@ -6,6 +6,6 @@ export default defineConfig({ retry: process.env.GITHUB_ACTIONS ? 3 : 0, disableConsoleIntercept: true, silent: false, - reporters: process.env.GITHUB_ACTIONS ? ['verbose', 'github-actions'] : ['verbose'], + reporters: process.env.GITHUB_ACTIONS || process.env.DEBUG ? ['basic', 'github-actions'] : ['verbose'], }, }); diff --git a/test/unit/offline-store.test.ts b/test/unit/offline-store.test.ts new file mode 100644 index 00000000..d929ad70 --- /dev/null +++ b/test/unit/offline-store.test.ts @@ -0,0 +1,100 @@ +import '../../scripts/electron-shim.mjs'; + +import { createEventEnvelope } from '@sentry/core'; +import { Envelope, Event } from '@sentry/types'; +import * as tmp from 'tmp'; +import { afterEach, beforeEach, describe, expect, test } from 'vitest'; + +import { delay, expectFilesInDirectory } from '../helpers'; + +const { createOfflineStore } = await import('../../src/main/transports/offline-store'); + +function EVENT_ENVELOPE(message: string = 'test', send_at?: Date) { + const env = createEventEnvelope({ message }); + if (send_at) { + env[0].sent_at = send_at.toISOString(); + } + return env; +} + +function getMessageFromEventEnvelope(envelope: Envelope | undefined): string | undefined { + return (envelope?.[1][0][1] as Event).message; +} + +describe('createOfflineStore', () => { + let tempDir: tmp.DirResult; + beforeEach(() => { + tempDir = tmp.dirSync({ unsafeCleanup: true }); + }); + + afterEach(() => { + if (tempDir) { + tempDir.removeCallback(); + } + }); + + test('Queues and returns a request', async () => { + const queue = createOfflineStore({ queuePath: tempDir.name }); + await expectFilesInDirectory(tempDir.name, 0); + + await queue.insert(EVENT_ENVELOPE()); + await delay(1_000); + await expectFilesInDirectory(tempDir.name, 2); + + // We create a new queue to force reading from serialized store + const queue2 = createOfflineStore({ queuePath: tempDir.name }); + const popped = await queue2.pop(); + expect(popped).to.not.be.undefined; + expect(getMessageFromEventEnvelope(popped)).toBe('test'); + + await delay(1_000); + await expectFilesInDirectory(tempDir.name, 1); + }); + + test('Drops requests when full', async () => { + const queue = createOfflineStore({ queuePath: tempDir.name, maxQueueSize: 5 }); + + await queue.insert(EVENT_ENVELOPE('1')); + await queue.insert(EVENT_ENVELOPE('2')); + await queue.insert(EVENT_ENVELOPE('3')); + await queue.insert(EVENT_ENVELOPE('4')); + await queue.insert(EVENT_ENVELOPE('5')); + await queue.insert(EVENT_ENVELOPE('6')); + await queue.insert(EVENT_ENVELOPE('7')); + + await delay(1_000); + await expectFilesInDirectory(tempDir.name, 6); + + const popped: Envelope[] = []; + let pop: Envelope | undefined; + while ((pop = await queue.pop())) { + popped.push(pop); + } + + expect(popped.length).to.equal(5); + expect(popped.map(getMessageFromEventEnvelope).join('')).to.equal('12345'); + + await delay(1_000); + await expectFilesInDirectory(tempDir.name, 1); + }); + + test('Drops old events', async () => { + const queue = createOfflineStore({ queuePath: tempDir.name, maxAgeDays: 1, maxQueueSize: 5 }); + + await queue.insert(EVENT_ENVELOPE('1', new Date(Date.now() - 100_000_000))); + await queue.insert(EVENT_ENVELOPE('2', new Date(Date.now() - 100_000_000))); + await queue.insert(EVENT_ENVELOPE('3', new Date(Date.now() - 100_000_000))); + await queue.insert(EVENT_ENVELOPE('4')); + + await delay(1_000); + await expectFilesInDirectory(tempDir.name, 2); + + const pop = await queue.pop(); + expect(pop).toBeDefined(); + const pop2 = await queue.pop(); + expect(pop2).toBeUndefined(); + + await delay(1_000); + await expectFilesInDirectory(tempDir.name, 1); + }); +}); diff --git a/test/unit/persist-queue.test.ts b/test/unit/persist-queue.test.ts deleted file mode 100644 index 7d06f8aa..00000000 --- a/test/unit/persist-queue.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import * as tmp from 'tmp'; -import { afterEach, beforeEach, describe, expect, test } from 'vitest'; - -import { PersistedRequestQueue, QueuedTransportRequest } from '../../src/main/transports/queue'; -import { delay, expectFilesInDirectory } from '../helpers'; - -describe('PersistedRequestQueue', () => { - let tempDir: tmp.DirResult; - beforeEach(() => { - tempDir = tmp.dirSync({ unsafeCleanup: true }); - }); - - afterEach(() => { - if (tempDir) { - tempDir.removeCallback(); - } - }); - - test('Queues and returns a request', async () => { - const queue = new PersistedRequestQueue(tempDir.name); - await expectFilesInDirectory(tempDir.name, 0); - - await queue.add({ body: 'just a string' }); - await delay(1_000); - await expectFilesInDirectory(tempDir.name, 2); - - // We create a new queue to force reading from serialized store - const queue2 = new PersistedRequestQueue(tempDir.name); - const popped = await queue2.pop(); - expect(popped).to.not.be.undefined; - expect(popped?.request?.date).to.be.instanceOf(Date); - expect(popped?.request?.body).to.not.be.undefined; - expect(popped?.request?.body.toString()).to.equal('just a string'); - - await delay(1_000); - await expectFilesInDirectory(tempDir.name, 1); - }); - - test('Correctly returns pending request count', async () => { - const queue = new PersistedRequestQueue(tempDir.name); - - const r1 = await queue.add({ body: 'just a string' }); - expect(r1).to.equal(1); - const r2 = await queue.add({ body: 'just another string' }); - expect(r2).to.equal(2); - - const r3 = await queue.pop(); - expect(r3?.pendingCount).to.equal(1); - - const r4 = await queue.pop(); - expect(r4?.pendingCount).to.equal(0); - }); - - test('Drops requests when full', async () => { - const queue = new PersistedRequestQueue(tempDir.name, 30, 5); - - await queue.add({ body: '1' }); - await queue.add({ body: '2' }); - await queue.add({ body: '3' }); - await queue.add({ body: '4' }); - await queue.add({ body: '5' }); - await queue.add({ body: '6' }); - await queue.add({ body: '7' }); - - await delay(1_000); - await expectFilesInDirectory(tempDir.name, 6); - - const popped: QueuedTransportRequest[] = []; - let pop: { request: QueuedTransportRequest } | undefined; - while ((pop = await queue.pop())) { - popped.push(pop.request); - } - - expect(popped.length).to.equal(5); - expect(popped.map((p) => p.body.toString()).join('')).to.equal('34567'); - - await delay(1_000); - await expectFilesInDirectory(tempDir.name, 1); - }); - - test('Drops old events', async () => { - const queue = new PersistedRequestQueue(tempDir.name, 1, 5); - - await queue.add({ body: 'so old 1', date: new Date(Date.now() - 100_000_000) }); - await queue.add({ body: 'so old 2', date: new Date(Date.now() - 100_000_000) }); - await queue.add({ body: 'so old 3', date: new Date(Date.now() - 100_000_000) }); - await queue.add({ body: 'so old 4' }); - - await delay(1_000); - await expectFilesInDirectory(tempDir.name, 5); - - const pop = await queue.pop(); - expect(pop).to.not.be.undefined; - const pop2 = await queue.pop(); - expect(pop2).to.be.undefined; - - await delay(1_000); - await expectFilesInDirectory(tempDir.name, 1); - }); -});