From b9a1c2b8768e98dc24ce5d0cceb1a54163326c5f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 4 Aug 2023 14:49:08 +0200 Subject: [PATCH] ref: Hoist `flush`, `close`, and `lastEventId` into `@sentry/core` (#8731) --- .github/workflows/build.yml | 2 +- packages/browser/src/exports.ts | 16 +++------ packages/browser/src/sdk.ts | 51 +---------------------------- packages/core/src/exports.ts | 43 ++++++++++++++++++++++++ packages/core/src/index.ts | 5 ++- packages/node/src/handlers.ts | 3 +- packages/node/src/index.ts | 5 ++- packages/node/src/sdk.ts | 44 ------------------------- packages/node/test/handlers.test.ts | 5 ++- 9 files changed, 61 insertions(+), 113 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 696ae9310d11..38de22f0868b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -599,7 +599,7 @@ jobs: needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_browser == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 - timeout-minutes: 10 + timeout-minutes: 20 strategy: fail-fast: false matrix: diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index f48b00c8c8e8..1f7117620c93 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -26,11 +26,14 @@ export { captureException, captureEvent, captureMessage, + close, configureScope, createTransport, + flush, getHubFromCarrier, getCurrentHub, Hub, + lastEventId, makeMain, Scope, startTransaction, @@ -60,16 +63,5 @@ export { } from './stack-parsers'; export { eventFromException, eventFromMessage } from './eventbuilder'; export { createUserFeedbackEnvelope } from './userfeedback'; -export { - defaultIntegrations, - forceLoad, - init, - lastEventId, - onLoad, - showReportDialog, - flush, - close, - wrap, - captureUserFeedback, -} from './sdk'; +export { defaultIntegrations, forceLoad, init, onLoad, showReportDialog, wrap, captureUserFeedback } from './sdk'; export { GlobalHandlers, TryCatch, Breadcrumbs, LinkedErrors, HttpContext, Dedupe } from './integrations'; diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 17fbfc159549..45fd26c75745 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -7,13 +7,7 @@ import { Integrations as CoreIntegrations, } from '@sentry/core'; import type { UserFeedback } from '@sentry/types'; -import { - addInstrumentationHandler, - logger, - resolvedSyncPromise, - stackParserFromStackParserOptions, - supportsFetch, -} from '@sentry/utils'; +import { addInstrumentationHandler, logger, stackParserFromStackParserOptions, supportsFetch } from '@sentry/utils'; import type { BrowserClientOptions, BrowserOptions } from './client'; import { BrowserClient } from './client'; @@ -179,15 +173,6 @@ export function showReportDialog(options: ReportDialogOptions = {}, hub: Hub = g } } -/** - * This is the getter for lastEventId. - * - * @returns The last event id of a captured event. - */ -export function lastEventId(): string | undefined { - return getCurrentHub().lastEventId(); -} - /** * This function is here to be API compatible with the loader. * @hidden @@ -204,40 +189,6 @@ export function onLoad(callback: () => void): void { callback(); } -/** - * Call `flush()` on the current client, if there is one. See {@link Client.flush}. - * - * @param timeout Maximum time in ms the client should wait to flush its event queue. Omitting this parameter will cause - * the client to wait until all events are sent before resolving the promise. - * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it - * doesn't (or if there's no client defined). - */ -export function flush(timeout?: number): PromiseLike { - const client = getCurrentHub().getClient(); - if (client) { - return client.flush(timeout); - } - __DEBUG_BUILD__ && logger.warn('Cannot flush events. No client defined.'); - return resolvedSyncPromise(false); -} - -/** - * Call `close()` on the current client, if there is one. See {@link Client.close}. - * - * @param timeout Maximum time in ms the client should wait to flush its event queue before shutting down. Omitting this - * parameter will cause the client to wait until all events are sent before disabling itself. - * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it - * doesn't (or if there's no client defined). - */ -export function close(timeout?: number): PromiseLike { - const client = getCurrentHub().getClient(); - if (client) { - return client.close(timeout); - } - __DEBUG_BUILD__ && logger.warn('Cannot flush events and disable SDK. No client defined.'); - return resolvedSyncPromise(false); -} - /** * Wrap code within a try/catch block so the SDK is able to capture errors. * diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index ad3d33013253..1a143a2efd4e 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -209,3 +209,46 @@ export function captureCheckIn(checkIn: CheckIn, upsertMonitorConfig?: MonitorCo return uuid4(); } + +/** + * Call `flush()` on the current client, if there is one. See {@link Client.flush}. + * + * @param timeout Maximum time in ms the client should wait to flush its event queue. Omitting this parameter will cause + * the client to wait until all events are sent before resolving the promise. + * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it + * doesn't (or if there's no client defined). + */ +export async function flush(timeout?: number): Promise { + const client = getCurrentHub().getClient(); + if (client) { + return client.flush(timeout); + } + __DEBUG_BUILD__ && logger.warn('Cannot flush events. No client defined.'); + return Promise.resolve(false); +} + +/** + * Call `close()` on the current client, if there is one. See {@link Client.close}. + * + * @param timeout Maximum time in ms the client should wait to flush its event queue before shutting down. Omitting this + * parameter will cause the client to wait until all events are sent before disabling itself. + * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it + * doesn't (or if there's no client defined). + */ +export async function close(timeout?: number): Promise { + const client = getCurrentHub().getClient(); + if (client) { + return client.close(timeout); + } + __DEBUG_BUILD__ && logger.warn('Cannot flush events and disable SDK. No client defined.'); + return Promise.resolve(false); +} + +/** + * This is the getter for lastEventId. + * + * @returns The last event id of a captured event. + */ +export function lastEventId(): string | undefined { + return getCurrentHub().lastEventId(); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9ff61f05cdb3..a0cc7e627bf7 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -5,10 +5,14 @@ export type { OfflineStore, OfflineTransportOptions } from './transports/offline export * from './tracing'; export { addBreadcrumb, + captureCheckIn, captureException, captureEvent, captureMessage, + close, configureScope, + flush, + lastEventId, startTransaction, setContext, setExtra, @@ -17,7 +21,6 @@ export { setTags, setUser, withScope, - captureCheckIn, } from './exports'; export { getCurrentHub, diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 1d1ef3bed507..5d28be6d3c21 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { captureException, + flush, getCurrentHub, hasTracingEnabled, runWithAsyncContext, @@ -25,7 +26,7 @@ import type { NodeClient } from './client'; import { extractRequestData } from './requestdata'; // TODO (v8 / XXX) Remove this import import type { ParseRequestOptions } from './requestDataDeprecated'; -import { flush, isAutoSessionTrackingEnabled } from './sdk'; +import { isAutoSessionTrackingEnabled } from './sdk'; /** * Express-compatible tracing handler. diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 1dd778e21199..0031a587c602 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -29,13 +29,16 @@ export { captureException, captureEvent, captureMessage, + close, configureScope, createTransport, extractTraceparentData, + flush, getActiveTransaction, getHubFromCarrier, getCurrentHub, Hub, + lastEventId, makeMain, runWithAsyncContext, Scope, @@ -57,7 +60,7 @@ export { autoDiscoverNodePerformanceMonitoringIntegrations } from './tracing'; export { NodeClient } from './client'; export { makeNodeTransport } from './transports'; -export { defaultIntegrations, init, defaultStackParser, lastEventId, flush, close, getSentryRelease } from './sdk'; +export { defaultIntegrations, init, defaultStackParser, getSentryRelease } from './sdk'; export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from './requestdata'; export { deepReadDirSync } from './utils'; export { getModuleFromFilename } from './module'; diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 7287d1938346..20e8160b7985 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -10,7 +10,6 @@ import type { SessionStatus, StackParser } from '@sentry/types'; import { createStackParser, GLOBAL_OBJ, - logger, nodeStackLineParser, stackParserFromStackParserOptions, tracingContextFromHeaders, @@ -177,49 +176,6 @@ export function init(options: NodeOptions = {}): void { updateScopeFromEnvVariables(); } -/** - * This is the getter for lastEventId. - * - * @returns The last event id of a captured event. - */ -export function lastEventId(): string | undefined { - return getCurrentHub().lastEventId(); -} - -/** - * Call `flush()` on the current client, if there is one. See {@link Client.flush}. - * - * @param timeout Maximum time in ms the client should wait to flush its event queue. Omitting this parameter will cause - * the client to wait until all events are sent before resolving the promise. - * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it - * doesn't (or if there's no client defined). - */ -export async function flush(timeout?: number): Promise { - const client = getCurrentHub().getClient(); - if (client) { - return client.flush(timeout); - } - __DEBUG_BUILD__ && logger.warn('Cannot flush events. No client defined.'); - return Promise.resolve(false); -} - -/** - * Call `close()` on the current client, if there is one. See {@link Client.close}. - * - * @param timeout Maximum time in ms the client should wait to flush its event queue before shutting down. Omitting this - * parameter will cause the client to wait until all events are sent before disabling itself. - * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it - * doesn't (or if there's no client defined). - */ -export async function close(timeout?: number): Promise { - const client = getCurrentHub().getClient(); - if (client) { - return client.close(timeout); - } - __DEBUG_BUILD__ && logger.warn('Cannot flush events and disable SDK. No client defined.'); - return Promise.resolve(false); -} - /** * Function that takes an instance of NodeClient and checks if autoSessionTracking option is enabled for that client */ diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index d464342fe396..e31383118c82 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -7,7 +7,6 @@ import * as http from 'http'; import { NodeClient } from '../src/client'; import { errorHandler, requestHandler, tracingHandler } from '../src/handlers'; -import * as SDK from '../src/sdk'; import { getDefaultNodeClientOptions } from './helper/node-client-options'; function mockAsyncContextStrategy(getHub: () => Hub): void { @@ -128,7 +127,7 @@ describe('requestHandler', () => { }); it('patches `res.end` when `flushTimeout` is specified', done => { - const flush = jest.spyOn(SDK, 'flush').mockResolvedValue(true); + const flush = jest.spyOn(sentryCore, 'flush').mockResolvedValue(true); const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 }); sentryRequestMiddleware(req, res, next); @@ -142,7 +141,7 @@ describe('requestHandler', () => { }); it('prevents errors thrown during `flush` from breaking the response', done => { - jest.spyOn(SDK, 'flush').mockRejectedValue(new SentryError('HTTP Error (429)')); + jest.spyOn(sentryCore, 'flush').mockRejectedValue(new SentryError('HTTP Error (429)')); const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 }); sentryRequestMiddleware(req, res, next);