Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(scope): Bring back lastEventId on isolation scope (#11951) #12022

Merged
merged 4 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 2 additions & 55 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1569,7 +1564,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()` |
Expand Down Expand Up @@ -1690,35 +1685,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:
Expand Down Expand Up @@ -1786,25 +1752,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`.
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export {
makeNodeTransport,
getDefaultIntegrations,
defaultStackParser,
lastEventId,
flush,
close,
getSentryRelease,
Expand Down
1 change: 1 addition & 0 deletions packages/aws-serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export {
makeNodeTransport,
NodeClient,
defaultStackParser,
lastEventId,
flush,
close,
getSentryRelease,
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export {
captureMessage,
close,
createTransport,
lastEventId,
flush,
// eslint-disable-next-line deprecation/deprecation
getCurrentHub,
Expand Down
1 change: 1 addition & 0 deletions packages/bun/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export {
makeNodeTransport,
NodeClient,
defaultStackParser,
lastEventId,
flush,
close,
getSentryRelease,
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {

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;
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export {
captureException,
captureEvent,
captureMessage,
lastEventId,
close,
flush,
setContext,
Expand Down
18 changes: 18 additions & 0 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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));

Expand All @@ -143,13 +147,27 @@ export class Scope implements ScopeInterface {
this._client = client;
}

/**
* @inheritDoc
*/
public setLastEventId(lastEventId: string | undefined): void {
this._lastEventId = lastEventId;
}

/**
* @inheritDoc
*/
public getClient<C extends Client>(): C | undefined {
return this._client as C | undefined;
}

/**
* @inheritDoc
*/
public lastEventId(): string | undefined {
return this._lastEventId;
}

/**
* @inheritDoc
*/
Expand Down
35 changes: 33 additions & 2 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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',
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions packages/deno/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export {
close,
createTransport,
continueTrace,
lastEventId,
flush,
getClient,
isInitialized,
Expand Down
1 change: 1 addition & 0 deletions packages/google-cloud-serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export {
makeNodeTransport,
NodeClient,
defaultStackParser,
lastEventId,
flush,
close,
getSentryRelease,
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export {
addBreadcrumb,
isInitialized,
getGlobalScope,
lastEventId,
close,
createTransport,
flush,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export {
makeNodeTransport,
getDefaultIntegrations,
defaultStackParser,
lastEventId,
flush,
close,
getSentryRelease,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/src/index.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
1 change: 1 addition & 0 deletions packages/sveltekit/src/index.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export declare const getCurrentHub: typeof clientSdk.getCurrentHub;

export declare function close(timeout?: number | undefined): PromiseLike<boolean>;
export declare function flush(timeout?: number | undefined): PromiseLike<boolean>;
export declare function lastEventId(): string | undefined;

export declare const continueTrace: typeof clientSdk.continueTrace;

Expand Down
1 change: 1 addition & 0 deletions packages/sveltekit/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export {
makeNodeTransport,
getDefaultIntegrations,
defaultStackParser,
lastEventId,
flush,
close,
getSentryRelease,
Expand Down
12 changes: 12 additions & 0 deletions packages/types/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ export interface Scope {
*/
getClient<C extends Client>(): 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
Expand Down
1 change: 1 addition & 0 deletions packages/vercel-edge/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export {
captureFeedback,
close,
createTransport,
lastEventId,
flush,
getClient,
isInitialized,
Expand Down
Loading