From 6f812629f70fbdc6324f963335fad40405ada9f1 Mon Sep 17 00:00:00 2001 From: Andrei <168741329+andreiborza@users.noreply.github.com> Date: Tue, 14 May 2024 17:50:46 +0200 Subject: [PATCH] feat(scope): Bring back `lastEventId` on isolation scope (#11951) (#12022) Co-authored-by: Lukas Stracke --- MIGRATION.md | 57 +------------------ packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/browser/src/exports.ts | 1 + packages/bun/src/index.ts | 1 + packages/core/src/baseclient.ts | 4 ++ packages/core/src/exports.ts | 15 +++++ packages/core/src/index.ts | 1 + packages/core/src/scope.ts | 18 ++++++ packages/core/test/lib/base.test.ts | 35 +++++++++++- packages/deno/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + packages/node/src/index.ts | 1 + packages/remix/src/index.server.ts | 1 + packages/remix/src/index.types.ts | 1 + packages/sveltekit/src/index.types.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + packages/types/src/scope.ts | 12 ++++ packages/vercel-edge/src/index.ts | 1 + 19 files changed, 97 insertions(+), 57 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 72bc88163953..edb03fa7f1c2 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -375,7 +375,7 @@ To make sure these integrations work properly you'll have to change how you ### General Removed top-level exports: `tracingOrigins`, `MetricsAggregator`, `metricsAggregatorIntegration`, `Severity`, -`Sentry.configureScope`, `Span`, `spanStatusfromHttpCode`, `makeMain`, `lastEventId`, `pushScope`, `popScope`, +`Sentry.configureScope`, `Span`, `spanStatusfromHttpCode`, `makeMain`, `pushScope`, `popScope`, `addGlobalEventProcessor`, `timestampWithMs`, `addExtensionMethods`, `addGlobalEventProcessor`, `getActiveTransaction` Removed `@sentry/utils` exports: `timestampWithMs`, `addOrUpdateIntegration`, `tracingContextFromHeaders`, `walk` @@ -389,7 +389,6 @@ Removed `@sentry/utils` exports: `timestampWithMs`, `addOrUpdateIntegration`, `t - [Removal of `Span` class export from SDK packages](./MIGRATION.md#removal-of-span-class-export-from-sdk-packages) - [Removal of `spanStatusfromHttpCode` in favour of `getSpanStatusFromHttpCode`](./MIGRATION.md#removal-of-spanstatusfromhttpcode-in-favour-of-getspanstatusfromhttpcode) - [Removal of `addGlobalEventProcessor` in favour of `addEventProcessor`](./MIGRATION.md#removal-of-addglobaleventprocessor-in-favour-of-addeventprocessor) -- [Removal of `lastEventId()` method](./MIGRATION.md#deprecate-lasteventid) - [Remove `void` from transport return types](./MIGRATION.md#remove-void-from-transport-return-types) - [Remove `addGlobalEventProcessor` in favor of `addEventProcessor`](./MIGRATION.md#remove-addglobaleventprocessor-in-favor-of-addeventprocessor) @@ -568,10 +567,6 @@ Sentry.getGlobalScope().addEventProcessor(event => { }); ``` -#### Removal of `lastEventId()` method - -The `lastEventId` function has been removed. See [below](./MIGRATION.md#deprecate-lasteventid) for more details. - #### Removal of `void` from transport return types The `send` method on the `Transport` interface now always requires a `TransportMakeRequestResponse` to be returned in @@ -1576,7 +1571,7 @@ If you are using the `Hub` right now, see the following table on how to migrate | captureException() | `Sentry.captureException()` | | captureMessage() | `Sentry.captureMessage()` | | captureEvent() | `Sentry.captureEvent()` | -| lastEventId() | REMOVED - Use event processors or beforeSend instead | +| lastEventId() | `Sentry.lastEventId()` | | addBreadcrumb() | `Sentry.addBreadcrumb()` | | setUser() | `Sentry.setUser()` | | setTags() | `Sentry.setTags()` | @@ -1697,35 +1692,6 @@ app.get('/your-route', req => { }); ``` -## Deprecate `Sentry.lastEventId()` and `hub.lastEventId()` - -`Sentry.lastEventId()` sometimes causes race conditions, so we are deprecating it in favour of the `beforeSend` -callback. - -```js -// Before -Sentry.init({ - beforeSend(event, hint) { - const lastCapturedEventId = Sentry.lastEventId(); - - // Do something with `lastCapturedEventId` here - - return event; - }, -}); - -// After -Sentry.init({ - beforeSend(event, hint) { - const lastCapturedEventId = event.event_id; - - // Do something with `lastCapturedEventId` here - - return event; - }, -}); -``` - ## Deprecated fields on `Span` and `Transaction` In v8, the Span class is heavily reworked. The following properties & methods are thus deprecated: @@ -1793,25 +1759,6 @@ Instead, import this directly from `@sentry/utils`. Generally, in most cases you should probably use `continueTrace` instead, which abstracts this away from you and handles scope propagation for you. -## Deprecate `lastEventId()` - -Instead, if you need the ID of a recently captured event, we recommend using `beforeSend` instead: - -```ts -import * as Sentry from '@sentry/browser'; - -Sentry.init({ - dsn: '__DSN__', - beforeSend(event, hint) { - const lastCapturedEventId = event.event_id; - - // Do something with `lastCapturedEventId` here - - return event; - }, -}); -``` - ## Deprecate `timestampWithMs` export - #7878 The `timestampWithMs` util is deprecated in favor of using `timestampInSeconds`. diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 28b75ee5145a..a4f3ca59fb1f 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -40,6 +40,7 @@ export { makeNodeTransport, getDefaultIntegrations, defaultStackParser, + lastEventId, flush, close, getSentryRelease, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index ade19b700fcd..1d2323df06e5 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -35,6 +35,7 @@ export { makeNodeTransport, NodeClient, defaultStackParser, + lastEventId, flush, close, getSentryRelease, diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 1b5a9d294144..6b8edb66541a 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -28,6 +28,7 @@ export { captureMessage, close, createTransport, + lastEventId, flush, // eslint-disable-next-line deprecation/deprecation getCurrentHub, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 3f0be5d7a914..fd7671b34b09 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -55,6 +55,7 @@ export { makeNodeTransport, NodeClient, defaultStackParser, + lastEventId, flush, close, getSentryRelease, diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 68a113e5ff2c..096b288c8a21 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -650,6 +650,10 @@ export abstract class BaseClient implements Client { this.emit('preprocessEvent', event, hint); + if (!event.type) { + isolationScope.setLastEventId(event.event_id || hint.event_id); + } + return prepareEvent(options, event, hint, currentScope, this, isolationScope).then(evt => { if (evt === null) { return evt; diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index 4ff223a20e60..8571e3ce1be0 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -120,6 +120,21 @@ export function setUser(user: User | null): void { getIsolationScope().setUser(user); } +/** + * The last error event id of the isolation scope. + * + * Warning: This function really returns the last recorded error event id on the current + * isolation scope. If you call this function after handling a certain error and another error + * is captured in between, the last one is returned instead of the one you might expect. + * Also, ids of events that were never sent to Sentry (for example because + * they were dropped in `beforeSend`) could be returned. + * + * @returns The last event id of the isolation scope. + */ +export function lastEventId(): string | undefined { + return getIsolationScope().lastEventId(); +} + /** * Create a cron monitor check in and send it to Sentry. * diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 75f8ceea4b6e..194b35b27153 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -15,6 +15,7 @@ export { captureException, captureEvent, captureMessage, + lastEventId, close, flush, setContext, diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 0cd761113abd..4bf17b936563 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -94,6 +94,9 @@ export class Scope implements ScopeInterface { /** The client on this scope */ protected _client?: Client; + /** Contains the last event id of a captured event. */ + protected _lastEventId?: string; + // NOTE: Any field which gets added here should get added not only to the constructor but also to the `clone` method. public constructor() { @@ -130,6 +133,7 @@ export class Scope implements ScopeInterface { newScope._sdkProcessingMetadata = { ...this._sdkProcessingMetadata }; newScope._propagationContext = { ...this._propagationContext }; newScope._client = this._client; + newScope._lastEventId = this._lastEventId; _setSpanForScope(newScope, _getSpanForScope(this)); @@ -143,6 +147,13 @@ export class Scope implements ScopeInterface { this._client = client; } + /** + * @inheritDoc + */ + public setLastEventId(lastEventId: string | undefined): void { + this._lastEventId = lastEventId; + } + /** * @inheritDoc */ @@ -150,6 +161,13 @@ export class Scope implements ScopeInterface { return this._client as C | undefined; } + /** + * @inheritDoc + */ + public lastEventId(): string | undefined { + return this._lastEventId; + } + /** * @inheritDoc */ diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 373c2d6424cc..3cfb230da5cf 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1,7 +1,15 @@ import type { Client, Envelope, ErrorEvent, Event, TransactionEvent } from '@sentry/types'; import { SentryError, SyncPromise, dsnToString, logger } from '@sentry/utils'; -import { Scope, addBreadcrumb, getCurrentScope, getIsolationScope, makeSession, setCurrentClient } from '../../src'; +import { + Scope, + addBreadcrumb, + getCurrentScope, + getIsolationScope, + lastEventId, + makeSession, + setCurrentClient, +} from '../../src'; import * as integrationModule from '../../src/integration'; import { TestClient, getDefaultTestClientOptions } from '../mocks/client'; import { AdHocIntegration, TestIntegration } from '../mocks/integration'; @@ -226,7 +234,6 @@ describe('BaseClient', () => { const client = new TestClient(options); client.captureException(new Error('test exception')); - expect(TestClient.instance!.event).toEqual( expect.objectContaining({ environment: 'production', @@ -244,6 +251,14 @@ describe('BaseClient', () => { ); }); + test('sets the correct lastEventId', () => { + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + + const eventId = client.captureException(new Error('test exception')); + expect(eventId).toEqual(lastEventId()); + }); + test('allows for providing explicit scope', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); @@ -343,6 +358,14 @@ describe('BaseClient', () => { ); }); + test('sets the correct lastEventId', () => { + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + + const eventId = client.captureMessage('test message'); + expect(eventId).toEqual(lastEventId()); + }); + test('should call `eventFromException` if input to `captureMessage` is not a primitive', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); @@ -444,6 +467,14 @@ describe('BaseClient', () => { ); }); + test('sets the correct lastEventId', () => { + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + + const eventId = client.captureEvent({ message: 'message' }, undefined); + expect(eventId).toEqual(lastEventId()); + }); + test('does not overwrite existing timestamp', () => { expect.assertions(2); diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index a46c052ec2a9..1857de352798 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -30,6 +30,7 @@ export { close, createTransport, continueTrace, + lastEventId, flush, getClient, isInitialized, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 79c39e74e870..067c27818a10 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -35,6 +35,7 @@ export { makeNodeTransport, NodeClient, defaultStackParser, + lastEventId, flush, close, getSentryRelease, diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 9854d30b4787..5cc16772189b 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -57,6 +57,7 @@ export { addBreadcrumb, isInitialized, getGlobalScope, + lastEventId, close, createTransport, flush, diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index ff00f7a80e58..ab35b4bf4847 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -43,6 +43,7 @@ export { makeNodeTransport, getDefaultIntegrations, defaultStackParser, + lastEventId, flush, close, getSentryRelease, diff --git a/packages/remix/src/index.types.ts b/packages/remix/src/index.types.ts index 85bbb1a56235..d573fd55bd9a 100644 --- a/packages/remix/src/index.types.ts +++ b/packages/remix/src/index.types.ts @@ -30,5 +30,6 @@ export declare const continueTrace: typeof clientSdk.continueTrace; export const close = runtime === 'client' ? clientSdk.close : serverSdk.close; export const flush = runtime === 'client' ? clientSdk.flush : serverSdk.flush; +export const lastEventId = runtime === 'client' ? clientSdk.lastEventId : serverSdk.lastEventId; export declare const metrics: typeof clientSdk.metrics & typeof serverSdk.metrics; diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index 81d1ac96aa2e..f2392a276ef4 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -48,6 +48,7 @@ export declare const getCurrentHub: typeof clientSdk.getCurrentHub; export declare function close(timeout?: number | undefined): PromiseLike; export declare function flush(timeout?: number | undefined): PromiseLike; +export declare function lastEventId(): string | undefined; export declare const continueTrace: typeof clientSdk.continueTrace; diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 8f74221a1c64..d7d50f64481e 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -36,6 +36,7 @@ export { makeNodeTransport, getDefaultIntegrations, defaultStackParser, + lastEventId, flush, close, getSentryRelease, diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index 83a03de45125..ccce883d3366 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -59,6 +59,18 @@ export interface Scope { */ getClient(): C | undefined; + /** + * Sets the last event id on the scope. + * @param lastEventId The last event id of a captured event. + */ + setLastEventId(lastEventId: string | undefined): void; + + /** + * This is the getter for lastEventId. + * @returns The last event id of a captured event. + */ + lastEventId(): string | undefined; + /** * Add internal on change listener. Used for sub SDKs that need to store the scope. * @hidden diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 8fbcebf40e8c..ce4ef113908b 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -30,6 +30,7 @@ export { captureFeedback, close, createTransport, + lastEventId, flush, getClient, isInitialized,