From a9efe488b4b5bf05a7936beae9086aa5405a2c92 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 13 Dec 2023 10:39:22 +0000 Subject: [PATCH 1/7] meta(eslint-config): Don't allow voiding Promises --- packages/eslint-config-sdk/src/base.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-config-sdk/src/base.js b/packages/eslint-config-sdk/src/base.js index 982d5887c51d..d43f2027051c 100644 --- a/packages/eslint-config-sdk/src/base.js +++ b/packages/eslint-config-sdk/src/base.js @@ -84,7 +84,7 @@ module.exports = { '@typescript-eslint/no-unused-expressions': ['error', { allowShortCircuit: true }], // Make sure Promises are handled appropriately - '@typescript-eslint/no-floating-promises': 'error', + '@typescript-eslint/no-floating-promises': ['error', { ignoreVoid: false }], // Disallow delete operator. We should make this operation opt in (by disabling this rule). '@typescript-eslint/no-dynamic-delete': 'error', From 6f65689de2842d64138354546de716cdc0ade495 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 18 Dec 2023 13:59:54 +0000 Subject: [PATCH 2/7] Fix all floating promises --- .../utils/replayHelpers.ts | 3 +- packages/browser/src/client.ts | 10 ++++-- .../browser/src/profiling/hubextensions.ts | 3 +- packages/core/src/baseclient.ts | 10 ++++-- packages/core/src/server-runtime-client.ts | 6 +++- .../deno/src/integrations/globalhandlers.ts | 4 +-- packages/e2e-tests/prepare.ts | 3 +- packages/e2e-tests/run.ts | 3 +- packages/eslint-config-sdk/src/base.js | 13 ++++++- packages/integrations/scripts/buildBundles.ts | 3 +- .../nextjs/src/common/utils/responseEnd.ts | 4 +-- .../common/withServerActionInstrumentation.ts | 4 ++- .../src/common/wrapApiHandlerWithSentry.ts | 6 ++-- .../common/wrapServerComponentWithSentry.ts | 4 ++- packages/node/src/anr/index.ts | 14 +++++--- .../src/coreHandlers/handleAfterSendEvent.ts | 6 +++- .../src/coreHandlers/handleGlobalEvent.ts | 4 ++- .../coreHandlers/handleNetworkBreadcrumbs.ts | 8 +++-- .../coreHandlers/util/addBreadcrumbEvent.ts | 27 +++++++++------ .../util/addFeedbackBreadcrumb.ts | 28 +++++++++------ .../EventBufferCompressionWorker.ts | 6 +++- packages/replay/src/replay.ts | 34 +++++++++++++------ packages/replay/src/util/addEvent.ts | 4 ++- .../replay/src/util/addGlobalListeners.ts | 7 ++-- .../replay/src/util/handleRecordingEmit.ts | 4 ++- packages/vercel-edge/src/transports/index.ts | 4 +-- 26 files changed, 154 insertions(+), 68 deletions(-) diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index b487342f5593..7c4e6d624ed3 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -113,7 +113,8 @@ export function waitForReplayRequests( const responses: Response[] = []; return new Promise(resolve => { - void page.waitForResponse( + // eslint-disable-next-line @typescript-eslint/no-floating-promises + page.waitForResponse( res => { const req = res.request(); diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 4e46314f6711..c8d4e4ce2f08 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -106,7 +106,10 @@ export class BrowserClient extends BaseClient { dsn: this.getDsn(), tunnel: this.getOptions().tunnel, }); - void this._sendEnvelope(envelope); + + // _sendEnvelope should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._sendEnvelope(envelope); } /** @@ -137,6 +140,9 @@ export class BrowserClient extends BaseClient { DEBUG_BUILD && logger.log('Sending outcomes:', outcomes); const envelope = createClientReportEnvelope(outcomes, this._options.tunnel && dsnToString(this._dsn)); - void this._sendEnvelope(envelope); + + // _sendEnvelope should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._sendEnvelope(envelope); } } diff --git a/packages/browser/src/profiling/hubextensions.ts b/packages/browser/src/profiling/hubextensions.ts index 99e2772c8f25..f042ae016609 100644 --- a/packages/browser/src/profiling/hubextensions.ts +++ b/packages/browser/src/profiling/hubextensions.ts @@ -139,7 +139,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio ); } // If the timeout exceeds, we want to stop profiling, but not finish the transaction - void onProfileHandler(); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + onProfileHandler(); }, MAX_PROFILE_DURATION_MS); // We need to reference the original finish call to avoid creating an infinite loop diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index c5d9fff7df6f..4b7ddf33b6d0 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -375,7 +375,10 @@ export abstract class BaseClient implements Client { */ public sendSession(session: Session | SessionAggregates): void { const env = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel); - void this._sendEnvelope(env); + + // _sendEnvelope should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._sendEnvelope(env); } /** @@ -409,7 +412,10 @@ export abstract class BaseClient implements Client { this._options._metadata, this._options.tunnel, ); - void this._sendEnvelope(metricsEnvelope); + + // _sendEnvelope should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._sendEnvelope(metricsEnvelope); } // Keep on() & emit() signatures in sync with types' client.ts interface diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index 719e2b81f086..9cffaca15faf 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -193,7 +193,11 @@ export class ServerRuntimeClient< ); DEBUG_BUILD && logger.info('Sending checkin:', checkIn.monitorSlug, checkIn.status); - void this._sendEnvelope(envelope); + + // _sendEnvelope should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._sendEnvelope(envelope); + return id; } diff --git a/packages/deno/src/integrations/globalhandlers.ts b/packages/deno/src/integrations/globalhandlers.ts index 9914764d4c45..09dc36e00fb7 100644 --- a/packages/deno/src/integrations/globalhandlers.ts +++ b/packages/deno/src/integrations/globalhandlers.ts @@ -87,7 +87,7 @@ function installGlobalErrorHandler(): void { data.preventDefault(); isExiting = true; - void flush().then(() => { + void flush().finally(() => { // rethrow to replicate Deno default behavior throw error; }); @@ -130,7 +130,7 @@ function installGlobalUnhandledRejectionHandler(): void { e.preventDefault(); isExiting = true; - void flush().then(() => { + flush().finally(() => { // rethrow to replicate Deno default behavior throw error; }); diff --git a/packages/e2e-tests/prepare.ts b/packages/e2e-tests/prepare.ts index 7fee7677bf94..9d9c233f580d 100644 --- a/packages/e2e-tests/prepare.ts +++ b/packages/e2e-tests/prepare.ts @@ -21,4 +21,5 @@ async function run(): Promise { } } -void run(); +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/packages/e2e-tests/run.ts b/packages/e2e-tests/run.ts index bdc269a867a8..a5002d7bcff9 100644 --- a/packages/e2e-tests/run.ts +++ b/packages/e2e-tests/run.ts @@ -80,4 +80,5 @@ async function run(): Promise { } } -void run(); +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/packages/eslint-config-sdk/src/base.js b/packages/eslint-config-sdk/src/base.js index d43f2027051c..a76af21e7a51 100644 --- a/packages/eslint-config-sdk/src/base.js +++ b/packages/eslint-config-sdk/src/base.js @@ -158,7 +158,17 @@ module.exports = { env: { jest: true, }, - files: ['test.ts', '*.test.ts', '*.test.tsx', '*.test.js', '*.test.jsx', 'test/**/*.ts', 'test/**/*.js'], + files: [ + 'test.ts', + '*.test.ts', + '*.test.tsx', + '*.test.js', + '*.test.jsx', + 'test/**/*.ts', + 'test/**/*.js', + 'tests/**/*.ts', + 'tests/**/*.js', + ], rules: { 'max-lines': 'off', '@typescript-eslint/explicit-function-return-type': 'off', @@ -171,6 +181,7 @@ module.exports = { '@typescript-eslint/no-empty-function': 'off', '@sentry-internal/sdk/no-optional-chaining': 'off', '@sentry-internal/sdk/no-nullish-coalescing': 'off', + '@typescript-eslint/no-floating-promises': 'off', }, }, { diff --git a/packages/integrations/scripts/buildBundles.ts b/packages/integrations/scripts/buildBundles.ts index 98a2a3456300..c3c61ed66ef3 100644 --- a/packages/integrations/scripts/buildBundles.ts +++ b/packages/integrations/scripts/buildBundles.ts @@ -47,7 +47,8 @@ if (runParallel) { process.exit(1); }); } else { - void (async () => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + (async () => { for (const integration of getIntegrations()) { await buildBundle(integration, 'es5'); await buildBundle(integration, 'es6'); diff --git a/packages/nextjs/src/common/utils/responseEnd.ts b/packages/nextjs/src/common/utils/responseEnd.ts index 4d0fc40082ec..64a3d2c59c52 100644 --- a/packages/nextjs/src/common/utils/responseEnd.ts +++ b/packages/nextjs/src/common/utils/responseEnd.ts @@ -26,7 +26,7 @@ import type { ResponseEndMethod, WrappedResponseEndMethod } from '../types'; export function autoEndTransactionOnResponseEnd(transaction: Transaction, res: ServerResponse): void { const wrapEndMethod = (origEnd: ResponseEndMethod): WrappedResponseEndMethod => { return function sentryWrappedEnd(this: ServerResponse, ...args: unknown[]) { - void finishTransaction(transaction, this); + finishTransaction(transaction, this); return origEnd.call(this, ...args); }; }; @@ -39,7 +39,7 @@ export function autoEndTransactionOnResponseEnd(transaction: Transaction, res: S } /** Finish the given response's transaction and set HTTP status data */ -export async function finishTransaction(transaction: Transaction | undefined, res: ServerResponse): Promise { +export function finishTransaction(transaction: Transaction | undefined, res: ServerResponse): void { if (transaction) { transaction.setHttpStatus(res.statusCode); transaction.finish(); diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index d87429ad528c..d850b3e362f9 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -129,7 +129,9 @@ async function withServerActionInstrumentationImplementation any> } }, () => { - void flushQueue(); + // flushQueue should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue(); }, ); diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts index 32117f21372b..709b68a6a158 100644 --- a/packages/node/src/anr/index.ts +++ b/packages/node/src/anr/index.ts @@ -183,10 +183,16 @@ function handleChildProcess(options: Options): void { captureEvent(createAnrEvent(options.anrThreshold, frames)); - void flush(3000).then(() => { - // We only capture one event to avoid spamming users with errors - process.exit(); - }); + flush(3000).then( + () => { + // We only capture one event to avoid spamming users with errors + process.exit(); + }, + () => { + // Flush can reject and we want to fail gracefully + process.exit(); + }, + ); } addEventProcessor(event => { diff --git a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts index 5cc2adf77a59..9936e501ec9b 100644 --- a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts +++ b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts @@ -1,5 +1,7 @@ import { getClient } from '@sentry/core'; import type { ErrorEvent, Event, TransactionEvent, Transport, TransportMakeRequestResponse } from '@sentry/types'; +import { logger } from '@sentry/utils'; +import { DEBUG_BUILD } from '../debug-build'; import type { ReplayContainer } from '../types'; import { isErrorEvent, isTransactionEvent } from '../util/eventUtils'; @@ -74,7 +76,9 @@ function handleErrorEvent(replay: ReplayContainer, event: ErrorEvent): void { setTimeout(() => { // Capture current event buffer as new replay - void replay.sendBufferedReplayOrFlush(); + replay.sendBufferedReplayOrFlush().then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Sending buffered replay failed.', e); + }); }); } diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 3dbe1498f987..1faeadffc132 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -44,7 +44,9 @@ export function handleGlobalEventListener( } if (isFeedbackEvent(event)) { - void replay.flush(); + replay.flush().then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Flushing replay failed.', e); + }); event.contexts.feedback.replay_id = replay.getSessionId(); // Add a replay breadcrumb for this piece of feedback addFeedbackBreadcrumb(replay, event); diff --git a/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts b/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts index 1ee12c7f5fe0..4a201ae9b65e 100644 --- a/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts +++ b/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts @@ -79,7 +79,9 @@ export function beforeAddNetworkBreadcrumb( // So any async mutations to it will not be reflected in the final breadcrumb enrichXhrBreadcrumb(breadcrumb, hint, options); - void captureXhrBreadcrumbToReplay(breadcrumb, hint, options); + // This call should not reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + captureXhrBreadcrumbToReplay(breadcrumb, hint, options); } if (_isFetchBreadcrumb(breadcrumb) && _isFetchHint(hint)) { @@ -88,7 +90,9 @@ export function beforeAddNetworkBreadcrumb( // So any async mutations to it will not be reflected in the final breadcrumb enrichFetchBreadcrumb(breadcrumb, hint, options); - void captureFetchBreadcrumbToReplay(breadcrumb, hint, options); + // This call should not reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + captureFetchBreadcrumbToReplay(breadcrumb, hint, options); } } catch (e) { DEBUG_BUILD && logger.warn('Error when enriching network breadcrumb'); diff --git a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts index 947fb12f1ae4..39becdbfde26 100644 --- a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts +++ b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts @@ -1,6 +1,7 @@ import { EventType } from '@sentry-internal/rrweb'; import type { Breadcrumb } from '@sentry/types'; -import { normalize } from '@sentry/utils'; +import { logger, normalize } from '@sentry/utils'; +import { DEBUG_BUILD } from '../../debug-build'; import type { ReplayContainer } from '../../types'; @@ -19,16 +20,20 @@ export function addBreadcrumbEvent(replay: ReplayContainer, breadcrumb: Breadcru } replay.addUpdate(() => { - void replay.throttledAddEvent({ - type: EventType.Custom, - // TODO: We were converting from ms to seconds for breadcrumbs, spans, - // but maybe we should just keep them as milliseconds - timestamp: (breadcrumb.timestamp || 0) * 1000, - data: { - tag: 'breadcrumb', - // normalize to max. 10 depth and 1_000 properties per object - payload: normalize(breadcrumb, 10, 1_000), - }, + Promise.resolve( + replay.throttledAddEvent({ + type: EventType.Custom, + // TODO: We were converting from ms to seconds for breadcrumbs, spans, + // but maybe we should just keep them as milliseconds + timestamp: (breadcrumb.timestamp || 0) * 1000, + data: { + tag: 'breadcrumb', + // normalize to max. 10 depth and 1_000 properties per object + payload: normalize(breadcrumb, 10, 1_000), + }, + }), + ).then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Adding breadcrumb event failed.', e); }); // Do not flush after console log messages diff --git a/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts b/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts index 5f2808d9b57d..8e84c731e236 100644 --- a/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts +++ b/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts @@ -1,5 +1,7 @@ import { EventType } from '@sentry-internal/rrweb'; import type { FeedbackEvent } from '@sentry/types'; +import { logger } from '@sentry/utils'; +import { DEBUG_BUILD } from '../../debug-build'; import type { ReplayContainer } from '../../types'; @@ -15,19 +17,23 @@ export function addFeedbackBreadcrumb(replay: ReplayContainer, event: FeedbackEv return true; } - void replay.throttledAddEvent({ - type: EventType.Custom, - timestamp: event.timestamp * 1000, - data: { - timestamp: event.timestamp, - tag: 'breadcrumb', - payload: { - category: 'sentry.feedback', - data: { - feedbackId: event.event_id, + Promise.resolve( + replay.throttledAddEvent({ + type: EventType.Custom, + timestamp: event.timestamp * 1000, + data: { + timestamp: event.timestamp, + tag: 'breadcrumb', + payload: { + category: 'sentry.feedback', + data: { + feedbackId: event.event_id, + }, }, }, - }, + }), + ).then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Adding feedback breadcrumb failed.', e); }); return false; diff --git a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts index 6e707638386f..a7fb8662c243 100644 --- a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts @@ -1,6 +1,8 @@ import type { ReplayRecordingData } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants'; +import { DEBUG_BUILD } from '../debug-build'; import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { timestampToMs } from '../util/timestamp'; import { WorkerHandler } from './WorkerHandler'; @@ -85,7 +87,9 @@ export class EventBufferCompressionWorker implements EventBuffer { this.hasCheckout = false; // We do not wait on this, as we assume the order of messages is consistent for the worker - void this._worker.postMessage('clear'); + this._worker.postMessage('clear').then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Clearing the event buffer failed.', e); + }); } /** @inheritdoc */ diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 0085c44c6eb8..99517cc99362 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -785,7 +785,9 @@ export class ReplayContainer implements ReplayContainerInterface { maxReplayDuration: this._options.maxReplayDuration, }) ) { - void this._refreshSession(currentSession); + this._refreshSession(currentSession).then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Refreshing the session failed.', e); + }); return false; } @@ -924,7 +926,9 @@ export class ReplayContainer implements ReplayContainerInterface { // Send replay when the page/tab becomes hidden. There is no reason to send // replay if it becomes visible, since no actions we care about were done // while it was hidden - void this.conditionalFlush(); + this.conditionalFlush().then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Background flushing failed.', e); + }); } /** @@ -972,13 +976,17 @@ export class ReplayContainer implements ReplayContainerInterface { */ private _createCustomBreadcrumb(breadcrumb: ReplayBreadcrumbFrame): void { this.addUpdate(() => { - void this.throttledAddEvent({ - type: EventType.Custom, - timestamp: breadcrumb.timestamp || 0, - data: { - tag: 'breadcrumb', - payload: breadcrumb, - }, + Promise.resolve( + this.throttledAddEvent({ + type: EventType.Custom, + timestamp: breadcrumb.timestamp || 0, + data: { + tag: 'breadcrumb', + payload: breadcrumb, + }, + }), + ).then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Adding custom breadcrumb failed.', e); }); }); } @@ -1113,7 +1121,9 @@ export class ReplayContainer implements ReplayContainerInterface { // This means we retried 3 times and all of them failed, // or we ran into a problem we don't want to retry, like rate limiting. // In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments - void this.stop({ reason: 'sendReplay' }); + this.stop({ reason: 'sendReplay' }).then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Stopping the replay failed.', e); + }); const client = getClient(); @@ -1237,7 +1247,9 @@ export class ReplayContainer implements ReplayContainerInterface { // Stop replay if over the mutation limit if (overMutationLimit) { - void this.stop({ reason: 'mutationLimit', forceFlush: this.recordingMode === 'session' }); + this.stop({ reason: 'mutationLimit', forceFlush: this.recordingMode === 'session' }).then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Stopping the replay because due to mutation limit failed.', e); + }); return false; } diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 7ea5d165eac5..4c671778c340 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -25,7 +25,9 @@ export function addEventSync(replay: ReplayContainer, event: RecordingEvent, isC return false; } - void _addEvent(replay, event, isCheckout); + _addEvent(replay, event, isCheckout).then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Adding event to the event buffer failed.', e); + }); return true; } diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index 1824e1fa606c..1b8a9c163ab4 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -2,7 +2,7 @@ import type { BaseClient } from '@sentry/core'; import { getCurrentScope } from '@sentry/core'; import { addEventProcessor, getClient } from '@sentry/core'; import type { Client, DynamicSamplingContext } from '@sentry/types'; -import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler } from '@sentry/utils'; +import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler, logger } from '@sentry/utils'; import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent'; import { handleBeforeSendEvent } from '../coreHandlers/handleBeforeSendEvent'; @@ -11,6 +11,7 @@ import { handleGlobalEventListener } from '../coreHandlers/handleGlobalEvent'; import { handleHistorySpanListener } from '../coreHandlers/handleHistory'; import { handleNetworkBreadcrumbs } from '../coreHandlers/handleNetworkBreadcrumbs'; import { handleScopeListener } from '../coreHandlers/handleScope'; +import { DEBUG_BUILD } from '../debug-build'; import type { ReplayContainer } from '../types'; /** @@ -65,7 +66,9 @@ export function addGlobalListeners(replay: ReplayContainer): void { client.on('beforeSendFeedback', (feedbackEvent, options) => { const replayId = replay.getSessionId(); if (options && options.includeReplay && replay.isEnabled() && replayId) { - void replay.flush(); + void replay.flush().then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Flushing replay failed.', e); + }); if (feedbackEvent.contexts && feedbackEvent.contexts.feedback) { feedbackEvent.contexts.feedback.replay_id = replayId; } diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index 3ab420a717b1..817f61625a90 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -100,7 +100,9 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa // a previous session ID. In this case, we want to buffer events // for a set amount of time before flushing. This can help avoid // capturing replays of users that immediately close the window. - void replay.flush(); + replay.flush().then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Flushing replay failed.', e); + }); } return true; diff --git a/packages/vercel-edge/src/transports/index.ts b/packages/vercel-edge/src/transports/index.ts index d73e7fd4341b..c6bea29df797 100644 --- a/packages/vercel-edge/src/transports/index.ts +++ b/packages/vercel-edge/src/transports/index.ts @@ -60,13 +60,13 @@ export class IsolatedPromiseBuffer { } }, timeout); - void Promise.all( + Promise.all( oldTaskProducers.map(taskProducer => taskProducer().then(null, () => { // catch all failed requests }), ), - ).then(() => { + ).finally(() => { // resolve to true if all fetch requests settled clearTimeout(timer); resolve(true); From 2ccd314faab399f60d3b3e2f1c4855ab928a9a7a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 18 Dec 2023 14:50:33 +0000 Subject: [PATCH 3/7] fix more --- .../suites/tracing-new/apollo-graphql/scenario.ts | 3 ++- .../suites/tracing-new/auto-instrument/mongodb/scenario.ts | 3 ++- .../suites/tracing-new/prisma-orm/scenario.ts | 3 ++- .../suites/tracing/apollo-graphql/scenario.ts | 3 ++- .../suites/tracing/auto-instrument/mongodb/scenario.ts | 3 ++- .../suites/tracing/prisma-orm/scenario.ts | 3 ++- packages/node-integration-tests/utils/index.ts | 6 ++++-- packages/node-integration-tests/utils/run-tests.ts | 3 ++- packages/overhead-metrics/src/perf/network.ts | 6 ++++-- packages/remix/src/utils/instrumentServer.ts | 4 +++- packages/serverless/scripts/buildLambdaLayer.ts | 3 ++- packages/serverless/src/gcpfunction/cloud_events.ts | 3 ++- packages/serverless/src/gcpfunction/events.ts | 3 ++- packages/serverless/src/gcpfunction/http.ts | 3 ++- 14 files changed, 33 insertions(+), 16 deletions(-) diff --git a/packages/node-integration-tests/suites/tracing-new/apollo-graphql/scenario.ts b/packages/node-integration-tests/suites/tracing-new/apollo-graphql/scenario.ts index 7fdbfce0351c..0c53294d1f4f 100644 --- a/packages/node-integration-tests/suites/tracing-new/apollo-graphql/scenario.ts +++ b/packages/node-integration-tests/suites/tracing-new/apollo-graphql/scenario.ts @@ -31,7 +31,8 @@ const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'tra Sentry.getCurrentScope().setSpan(transaction); -void (async () => { +// eslint-disable-next-line @typescript-eslint/no-floating-promises +(async () => { // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: '{hello}', diff --git a/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/scenario.ts b/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/scenario.ts index cae4627e7096..9d747e2eff4b 100644 --- a/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/scenario.ts +++ b/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/scenario.ts @@ -41,4 +41,5 @@ async function run(): Promise { } } -void run(); +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/packages/node-integration-tests/suites/tracing-new/prisma-orm/scenario.ts b/packages/node-integration-tests/suites/tracing-new/prisma-orm/scenario.ts index c7a5ef761a82..ee73dc922747 100644 --- a/packages/node-integration-tests/suites/tracing-new/prisma-orm/scenario.ts +++ b/packages/node-integration-tests/suites/tracing-new/prisma-orm/scenario.ts @@ -42,4 +42,5 @@ async function run(): Promise { } } -void run(); +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/packages/node-integration-tests/suites/tracing/apollo-graphql/scenario.ts b/packages/node-integration-tests/suites/tracing/apollo-graphql/scenario.ts index 4a4d5a989227..0e5e0bd9edd0 100644 --- a/packages/node-integration-tests/suites/tracing/apollo-graphql/scenario.ts +++ b/packages/node-integration-tests/suites/tracing/apollo-graphql/scenario.ts @@ -33,7 +33,8 @@ const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'tra Sentry.getCurrentScope().setSpan(transaction); -void (async () => { +// eslint-disable-next-line @typescript-eslint/no-floating-promises +(async () => { // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: '{hello}', diff --git a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts index 5bd16772d50f..7979ea57483b 100644 --- a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts +++ b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts @@ -42,4 +42,5 @@ async function run(): Promise { } } -void run(); +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/packages/node-integration-tests/suites/tracing/prisma-orm/scenario.ts b/packages/node-integration-tests/suites/tracing/prisma-orm/scenario.ts index 0014717b5fc4..578c5802fea0 100644 --- a/packages/node-integration-tests/suites/tracing/prisma-orm/scenario.ts +++ b/packages/node-integration-tests/suites/tracing/prisma-orm/scenario.ts @@ -44,4 +44,5 @@ async function run(): Promise { } } -void run(); +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/packages/node-integration-tests/utils/index.ts b/packages/node-integration-tests/utils/index.ts index ced5b2475aee..b2b81361c0ea 100644 --- a/packages/node-integration-tests/utils/index.ts +++ b/packages/node-integration-tests/utils/index.ts @@ -184,7 +184,8 @@ export class TestEnv { envelopeTypeArray, ); - void makeRequest(options.method, options.url || this.url, this._axiosConfig); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + makeRequest(options.method, options.url || this.url, this._axiosConfig); return resProm; } @@ -305,7 +306,8 @@ export class TestEnv { nock.cleanAll(); - void this._closeServer().then(() => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._closeServer().then(() => { resolve(reqCount); }); }, diff --git a/packages/node-integration-tests/utils/run-tests.ts b/packages/node-integration-tests/utils/run-tests.ts index 3ff976e18443..90b78bf2521c 100644 --- a/packages/node-integration-tests/utils/run-tests.ts +++ b/packages/node-integration-tests/utils/run-tests.ts @@ -70,7 +70,8 @@ const workers = os.cpus().map(async (_, i) => { } }); -void Promise.all(workers).then(() => { +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Promise.all(workers).then(() => { console.log('-------------------'); console.log(`Successfully ran ${numTests} tests.`); if (fails.length > 0) { diff --git a/packages/overhead-metrics/src/perf/network.ts b/packages/overhead-metrics/src/perf/network.ts index c6755f091ebf..03b76d2fcc4d 100644 --- a/packages/overhead-metrics/src/perf/network.ts +++ b/packages/overhead-metrics/src/perf/network.ts @@ -71,11 +71,13 @@ export class NetworkUsageCollector { this._events.push(event); // Note: playwright would error out on file:/// requests. They are used to access local test app resources. if (url.startsWith('file:///')) { - void route.continue(); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + route.continue(); } else { const response = await route.fetch(); const body = await response.body(); - void route.fulfill({ response, body }); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + route.fulfill({ response, body }); event.responseTimeNs = process.hrtime.bigint(); event.responseSize = body.length; } diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 816410fd75f9..961594374380 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -91,7 +91,9 @@ export function wrapRemixHandleError(err: unknown, { request }: DataFunctionArgs return; } - void captureRemixServerException(err, 'remix.server.handleError', request); + captureRemixServerException(err, 'remix.server.handleError', request).then(null, e => { + DEBUG_BUILD && logger.warn('Failed to capture Remix Server exception.', e); + }); } /** diff --git a/packages/serverless/scripts/buildLambdaLayer.ts b/packages/serverless/scripts/buildLambdaLayer.ts index 99140ccb0513..540a1cab7451 100644 --- a/packages/serverless/scripts/buildLambdaLayer.ts +++ b/packages/serverless/scripts/buildLambdaLayer.ts @@ -54,7 +54,8 @@ async function buildLambdaLayer(): Promise { run(`zip -r -y ${zipFilename} .`, { cwd: 'build/aws/dist-serverless' }); } -void buildLambdaLayer(); +// eslint-disable-next-line @typescript-eslint/no-floating-promises +buildLambdaLayer(); /** * Make a directory synchronously, overwriting the old directory if necessary. diff --git a/packages/serverless/src/gcpfunction/cloud_events.ts b/packages/serverless/src/gcpfunction/cloud_events.ts index 63303470d9e9..0e89216a2fd7 100644 --- a/packages/serverless/src/gcpfunction/cloud_events.ts +++ b/packages/serverless/src/gcpfunction/cloud_events.ts @@ -54,7 +54,8 @@ function _wrapCloudEventFunction( } transaction?.finish(); - void flush(options.flushTimeout) + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flush(options.flushTimeout) .then(null, e => { DEBUG_BUILD && logger.error(e); }) diff --git a/packages/serverless/src/gcpfunction/events.ts b/packages/serverless/src/gcpfunction/events.ts index 29d151593990..b69335b272d7 100644 --- a/packages/serverless/src/gcpfunction/events.ts +++ b/packages/serverless/src/gcpfunction/events.ts @@ -56,7 +56,8 @@ function _wrapEventFunction } transaction?.finish(); - void flush(options.flushTimeout) + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flush(options.flushTimeout) .then(null, e => { DEBUG_BUILD && logger.error(e); }) diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 95c84cafeb80..726f56c91a52 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -110,7 +110,8 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial { DEBUG_BUILD && logger.error(e); }) From d3e8dc025cbf796c497d101e638db982f67e73b2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 18 Dec 2023 15:09:50 +0000 Subject: [PATCH 4/7] . --- .../deno/src/integrations/globalhandlers.ts | 28 ++++++++---- .../src/common/wrapApiHandlerWithSentry.ts | 5 +-- .../src/coreHandlers/handleAfterSendEvent.ts | 8 ++-- .../src/coreHandlers/handleGlobalEvent.ts | 6 +-- .../coreHandlers/util/addBreadcrumbEvent.ts | 29 ++++++------ .../util/addFeedbackBreadcrumb.ts | 30 ++++++------- .../EventBufferCompressionWorker.ts | 8 ++-- packages/replay/src/replay.ts | 44 +++++++++---------- packages/replay/src/util/addEvent.ts | 6 +-- .../replay/src/util/addGlobalListeners.ts | 9 ++-- .../replay/src/util/handleRecordingEmit.ts | 7 +-- packages/vercel-edge/src/transports/index.ts | 4 +- 12 files changed, 92 insertions(+), 92 deletions(-) diff --git a/packages/deno/src/integrations/globalhandlers.ts b/packages/deno/src/integrations/globalhandlers.ts index 09dc36e00fb7..145798b48b09 100644 --- a/packages/deno/src/integrations/globalhandlers.ts +++ b/packages/deno/src/integrations/globalhandlers.ts @@ -87,10 +87,16 @@ function installGlobalErrorHandler(): void { data.preventDefault(); isExiting = true; - void flush().finally(() => { - // rethrow to replicate Deno default behavior - throw error; - }); + void flush().then( + () => { + // rethrow to replicate Deno default behavior + throw error; + }, + () => { + // rethrow to replicate Deno default behavior + throw error; + }, + ); }); } @@ -130,10 +136,16 @@ function installGlobalUnhandledRejectionHandler(): void { e.preventDefault(); isExiting = true; - flush().finally(() => { - // rethrow to replicate Deno default behavior - throw error; - }); + flush().then( + () => { + // rethrow to replicate Deno default behavior + throw error; + }, + () => { + // rethrow to replicate Deno default behavior + throw error; + }, + ); }); } diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index a061878c04a1..dfd6d888fbef 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -207,15 +207,14 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri res.statusCode = 500; res.statusMessage = 'Internal Server Error'; + finishTransaction(transaction, res); + // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already // be finished and the queue will already be empty, so effectively it'll just no-op.) if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - finishTransaction(transaction, res); - } else { - finishTransaction(transaction, res); await flushQueue(); } diff --git a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts index 9936e501ec9b..28e112e736f0 100644 --- a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts +++ b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts @@ -1,7 +1,5 @@ import { getClient } from '@sentry/core'; import type { ErrorEvent, Event, TransactionEvent, Transport, TransportMakeRequestResponse } from '@sentry/types'; -import { logger } from '@sentry/utils'; -import { DEBUG_BUILD } from '../debug-build'; import type { ReplayContainer } from '../types'; import { isErrorEvent, isTransactionEvent } from '../util/eventUtils'; @@ -76,9 +74,9 @@ function handleErrorEvent(replay: ReplayContainer, event: ErrorEvent): void { setTimeout(() => { // Capture current event buffer as new replay - replay.sendBufferedReplayOrFlush().then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Sending buffered replay failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + replay.sendBufferedReplayOrFlush(); }); } diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 1faeadffc132..39983a85d72e 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -44,9 +44,9 @@ export function handleGlobalEventListener( } if (isFeedbackEvent(event)) { - replay.flush().then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Flushing replay failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + replay.flush(); event.contexts.feedback.replay_id = replay.getSessionId(); // Add a replay breadcrumb for this piece of feedback addFeedbackBreadcrumb(replay, event); diff --git a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts index 39becdbfde26..a324e5b24b25 100644 --- a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts +++ b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts @@ -1,7 +1,6 @@ import { EventType } from '@sentry-internal/rrweb'; import type { Breadcrumb } from '@sentry/types'; -import { logger, normalize } from '@sentry/utils'; -import { DEBUG_BUILD } from '../../debug-build'; +import { normalize } from '@sentry/utils'; import type { ReplayContainer } from '../../types'; @@ -20,20 +19,18 @@ export function addBreadcrumbEvent(replay: ReplayContainer, breadcrumb: Breadcru } replay.addUpdate(() => { - Promise.resolve( - replay.throttledAddEvent({ - type: EventType.Custom, - // TODO: We were converting from ms to seconds for breadcrumbs, spans, - // but maybe we should just keep them as milliseconds - timestamp: (breadcrumb.timestamp || 0) * 1000, - data: { - tag: 'breadcrumb', - // normalize to max. 10 depth and 1_000 properties per object - payload: normalize(breadcrumb, 10, 1_000), - }, - }), - ).then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Adding breadcrumb event failed.', e); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + replay.throttledAddEvent({ + type: EventType.Custom, + // TODO: We were converting from ms to seconds for breadcrumbs, spans, + // but maybe we should just keep them as milliseconds + timestamp: (breadcrumb.timestamp || 0) * 1000, + data: { + tag: 'breadcrumb', + // normalize to max. 10 depth and 1_000 properties per object + payload: normalize(breadcrumb, 10, 1_000), + }, }); // Do not flush after console log messages diff --git a/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts b/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts index 8e84c731e236..0e08b459d3ca 100644 --- a/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts +++ b/packages/replay/src/coreHandlers/util/addFeedbackBreadcrumb.ts @@ -1,7 +1,5 @@ import { EventType } from '@sentry-internal/rrweb'; import type { FeedbackEvent } from '@sentry/types'; -import { logger } from '@sentry/utils'; -import { DEBUG_BUILD } from '../../debug-build'; import type { ReplayContainer } from '../../types'; @@ -17,23 +15,21 @@ export function addFeedbackBreadcrumb(replay: ReplayContainer, event: FeedbackEv return true; } - Promise.resolve( - replay.throttledAddEvent({ - type: EventType.Custom, - timestamp: event.timestamp * 1000, - data: { - timestamp: event.timestamp, - tag: 'breadcrumb', - payload: { - category: 'sentry.feedback', - data: { - feedbackId: event.event_id, - }, + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + replay.throttledAddEvent({ + type: EventType.Custom, + timestamp: event.timestamp * 1000, + data: { + timestamp: event.timestamp, + tag: 'breadcrumb', + payload: { + category: 'sentry.feedback', + data: { + feedbackId: event.event_id, }, }, - }), - ).then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Adding feedback breadcrumb failed.', e); + }, }); return false; diff --git a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts index a7fb8662c243..53ae02383e99 100644 --- a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts @@ -1,8 +1,6 @@ import type { ReplayRecordingData } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants'; -import { DEBUG_BUILD } from '../debug-build'; import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { timestampToMs } from '../util/timestamp'; import { WorkerHandler } from './WorkerHandler'; @@ -87,9 +85,9 @@ export class EventBufferCompressionWorker implements EventBuffer { this.hasCheckout = false; // We do not wait on this, as we assume the order of messages is consistent for the worker - this._worker.postMessage('clear').then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Clearing the event buffer failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._worker.postMessage('clear'); } /** @inheritdoc */ diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 99517cc99362..e051f3120e8a 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -785,9 +785,9 @@ export class ReplayContainer implements ReplayContainerInterface { maxReplayDuration: this._options.maxReplayDuration, }) ) { - this._refreshSession(currentSession).then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Refreshing the session failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._refreshSession(currentSession); return false; } @@ -926,9 +926,9 @@ export class ReplayContainer implements ReplayContainerInterface { // Send replay when the page/tab becomes hidden. There is no reason to send // replay if it becomes visible, since no actions we care about were done // while it was hidden - this.conditionalFlush().then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Background flushing failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + void this.conditionalFlush(); } /** @@ -976,17 +976,15 @@ export class ReplayContainer implements ReplayContainerInterface { */ private _createCustomBreadcrumb(breadcrumb: ReplayBreadcrumbFrame): void { this.addUpdate(() => { - Promise.resolve( - this.throttledAddEvent({ - type: EventType.Custom, - timestamp: breadcrumb.timestamp || 0, - data: { - tag: 'breadcrumb', - payload: breadcrumb, - }, - }), - ).then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Adding custom breadcrumb failed.', e); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this.throttledAddEvent({ + type: EventType.Custom, + timestamp: breadcrumb.timestamp || 0, + data: { + tag: 'breadcrumb', + payload: breadcrumb, + }, }); }); } @@ -1121,9 +1119,9 @@ export class ReplayContainer implements ReplayContainerInterface { // This means we retried 3 times and all of them failed, // or we ran into a problem we don't want to retry, like rate limiting. // In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments - this.stop({ reason: 'sendReplay' }).then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Stopping the replay failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this.stop({ reason: 'sendReplay' }); const client = getClient(); @@ -1247,9 +1245,9 @@ export class ReplayContainer implements ReplayContainerInterface { // Stop replay if over the mutation limit if (overMutationLimit) { - this.stop({ reason: 'mutationLimit', forceFlush: this.recordingMode === 'session' }).then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Stopping the replay because due to mutation limit failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this.stop({ reason: 'mutationLimit', forceFlush: this.recordingMode === 'session' }); return false; } diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 4c671778c340..893c6b7d01a4 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -25,9 +25,9 @@ export function addEventSync(replay: ReplayContainer, event: RecordingEvent, isC return false; } - _addEvent(replay, event, isCheckout).then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Adding event to the event buffer failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + _addEvent(replay, event, isCheckout); return true; } diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index 1b8a9c163ab4..85f253363c7a 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -2,7 +2,7 @@ import type { BaseClient } from '@sentry/core'; import { getCurrentScope } from '@sentry/core'; import { addEventProcessor, getClient } from '@sentry/core'; import type { Client, DynamicSamplingContext } from '@sentry/types'; -import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler, logger } from '@sentry/utils'; +import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler } from '@sentry/utils'; import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent'; import { handleBeforeSendEvent } from '../coreHandlers/handleBeforeSendEvent'; @@ -11,7 +11,6 @@ import { handleGlobalEventListener } from '../coreHandlers/handleGlobalEvent'; import { handleHistorySpanListener } from '../coreHandlers/handleHistory'; import { handleNetworkBreadcrumbs } from '../coreHandlers/handleNetworkBreadcrumbs'; import { handleScopeListener } from '../coreHandlers/handleScope'; -import { DEBUG_BUILD } from '../debug-build'; import type { ReplayContainer } from '../types'; /** @@ -66,9 +65,9 @@ export function addGlobalListeners(replay: ReplayContainer): void { client.on('beforeSendFeedback', (feedbackEvent, options) => { const replayId = replay.getSessionId(); if (options && options.includeReplay && replay.isEnabled() && replayId) { - void replay.flush().then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Flushing replay failed.', e); - }); + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + replay.flush(); if (feedbackEvent.contexts && feedbackEvent.contexts.feedback) { feedbackEvent.contexts.feedback.replay_id = replayId; } diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index 817f61625a90..1d0fd10d0aa3 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -100,9 +100,10 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa // a previous session ID. In this case, we want to buffer events // for a set amount of time before flushing. This can help avoid // capturing replays of users that immediately close the window. - replay.flush().then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Flushing replay failed.', e); - }); + + // This should never reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + void replay.flush(); } return true; diff --git a/packages/vercel-edge/src/transports/index.ts b/packages/vercel-edge/src/transports/index.ts index c6bea29df797..b2b899ecdb42 100644 --- a/packages/vercel-edge/src/transports/index.ts +++ b/packages/vercel-edge/src/transports/index.ts @@ -60,13 +60,15 @@ export class IsolatedPromiseBuffer { } }, timeout); + // This cannot reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises Promise.all( oldTaskProducers.map(taskProducer => taskProducer().then(null, () => { // catch all failed requests }), ), - ).finally(() => { + ).then(() => { // resolve to true if all fetch requests settled clearTimeout(timer); resolve(true); From 06d041a53dceecd171282f90d0860a782ba03a57 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 19 Dec 2023 08:42:55 +0000 Subject: [PATCH 5/7] catch promise --- .../src/eventBuffer/EventBufferCompressionWorker.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts index 53ae02383e99..21206ea652ac 100644 --- a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts @@ -1,6 +1,8 @@ import type { ReplayRecordingData } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants'; +import { DEBUG_BUILD } from '../debug-build'; import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { timestampToMs } from '../util/timestamp'; import { WorkerHandler } from './WorkerHandler'; @@ -85,9 +87,9 @@ export class EventBufferCompressionWorker implements EventBuffer { this.hasCheckout = false; // We do not wait on this, as we assume the order of messages is consistent for the worker - // This should never reject - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this._worker.postMessage('clear'); + this._worker.postMessage('clear').then(null, e => { + DEBUG_BUILD && logger.warn('[Replay] Sending "clear" message to worker failed', e); + }); } /** @inheritdoc */ From 1c187a3cc42ebaa7d9e0330c0d6e1643817f9542 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 20 Dec 2023 09:03:27 +0000 Subject: [PATCH 6/7] Review feedback --- .../utils/replayHelpers.ts | 42 ++++++++----------- .../deno/src/integrations/globalhandlers.ts | 2 +- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index 7c4e6d624ed3..613bd5b447f1 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -105,41 +105,35 @@ export function waitForReplayRequest( * Wait until a callback returns true, collecting all replay responses along the way. * This can be useful when you don't know if stuff will be in one or multiple replay requests. */ -export function waitForReplayRequests( +export async function waitForReplayRequests( page: Page, callback: (event: ReplayEvent, res: Response) => boolean, timeout?: number, ): Promise { const responses: Response[] = []; - return new Promise(resolve => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - page.waitForResponse( - res => { - const req = res.request(); + await page.waitForResponse( + res => { + const req = res.request(); - const event = getReplayEventFromRequest(req); + const event = getReplayEventFromRequest(req); - if (!event) { - return false; - } + if (!event) { + return false; + } - responses.push(res); + responses.push(res); - try { - if (callback(event, res)) { - resolve(responses); - return true; - } + try { + return callback(event, res); + } catch { + return false; + } + }, + timeout ? { timeout } : undefined, + ); - return false; - } catch { - return false; - } - }, - timeout ? { timeout } : undefined, - ); - }); + return responses; } export function isReplayEvent(event: Event): event is ReplayEvent { diff --git a/packages/deno/src/integrations/globalhandlers.ts b/packages/deno/src/integrations/globalhandlers.ts index 05b17f62a527..4160e3f4b3c6 100644 --- a/packages/deno/src/integrations/globalhandlers.ts +++ b/packages/deno/src/integrations/globalhandlers.ts @@ -79,7 +79,7 @@ function installGlobalErrorHandler(client: Client): void { data.preventDefault(); isExiting = true; - void flush().then( + flush().then( () => { // rethrow to replicate Deno default behavior throw error; From 6db7feecfcdc966997e4df304959b2ed6fbd1f25 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 20 Dec 2023 09:07:46 +0000 Subject: [PATCH 7/7] Fix anr --- packages/node/src/integrations/anr/worker.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index fd333314e3eb..142fe0d608e7 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -152,7 +152,9 @@ if (options.captureStackTrace) { session.post('Debugger.disable'); const context = trace_id?.length && span_id?.length ? { trace_id, span_id, parent_span_id } : undefined; - void sendAnrEvent(stackFrames, context); + sendAnrEvent(stackFrames, context).then(null, () => { + log('Sending ANR event failed.'); + }); }, ); } catch (e) { @@ -196,7 +198,9 @@ function watchdogTimeout(): void { debuggerPause(); } else { log('Capturing event without a stack trace'); - void sendAnrEvent(); + sendAnrEvent().then(null, () => { + log('Sending ANR event failed on watchdog timeout.'); + }); } }