lastEventId is wrong when errors are Deduped

[piotrblasiak]

Package + Version

• [x ] @sentry/browser
• @sentry/node
• raven-js
• raven-node (raven for node)
• other:

Version:

6.13.2

Description

I use the lastEventId as an error reference that I show to my users. Using that I can then find the error they experienced, but for some time now I have very often not been able to find the error - often thinking they were offline when it happened but now I found out that it is the Dedupe filter that simply blocks the event from being sent BUT it still reports a new lastEventId.
Is there a way to fix this, is it intended behaviour and how do I work around it if it is? To me, the logical thing would be to set lastEventId to the last event of that type that was sent.

[rhcarvalho]

@getsentry/team-webplatform I think we have to understand this issue as something more than the Dedupe integration. In fact, the lastEventId will not match the "last event sent" in several other cases, particularly when events are dropped by event processors or beforeSend.

Before we'd special-case the Dedupe integration as in #4021, I'd consider clarifying the meaning of lastEventId -- note that other SDKs also update the lastEventId regardless of events being discarded in the same fashion as the JS SDKs.

[rhcarvalho]

The team has discussed this outside of GitHub, publishing the outcome here so we know how to proceed:

• There is a consensus that lastEventId is best effort and not strictly correct in all cases (in particular when there is concurrency involved). Still the API is provided for the cases where is it still useful (when the ID cannot be easily obtained because an error was captured elsewhere).
• The ID is of the last event that was attempted to be delivered which is not a transaction. "Attempted to be delivered" means we store it on the hub if we pass it to the transport, as in the Python SDK implementation.
• The client is not supposed to return the ID if the event was dropped (sampling, event processors, beforeSend or any other reason), and thus the last ID stored in the hub should not change.
• If the event is dropped in the transport layer (queue overflow, network error, etc), the hub will not know about that an will end up with as lastEventId that was not ingested (similarly, even if there were no errors, keep in mind that events can still be dropped during server-side processing).
• Nothing specific to the Dedupe integration needs to change (though the behavior will change when we fix the other considerations above).

Action Items

(cc @onurtemizkan)

• Hub.Capture* should not immediately update lastEventId (first line).
  

  sentry-javascript/packages/hub/src/hub.ts

  Lines 188 to 214 in 5bba9b4

  	public captureException(exception: any, hint?: EventHint): string {
  	const eventId = (this._lastEventId = uuid4());
  	let finalHint = hint;
  	// If there's no explicit hint provided, mimic the same thing that would happen
  	// in the minimal itself to create a consistent behavior.
  	// We don't do this in the client, as it's the lowest level API, and doing this,
  	// would prevent user from having full control over direct calls.
  	if (!hint) {
  	let syntheticException: Error;
  	try {
  	throw new Error('Sentry syntheticException');
  	} catch (exception) {
  	syntheticException = exception as Error;
  	}
  	finalHint = {
  	originalException: exception,
  	syntheticException,
  	};
  	}
  	this._invokeClient('captureException', exception, {
  	...finalHint,
  	event_id: eventId,
  	});
  	return eventId;
  	}

• Client should not return an event ID if if knows the event was dropped.
  

  sentry-javascript/packages/core/src/baseclient.ts

  Lines 101 to 114 in efa6c45

  	public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined {
  	let eventId: string | undefined = hint && hint.event_id;
  	this._process(
  	this._getBackend()
  	.eventFromException(exception, hint)
  	.then(event => this._captureEvent(event, hint, scope))
  	.then(result => {
  	eventId = result;
  	}),
  	);
  	return eventId;
  	}

• Compare other parts of the implementation to Python, make sure the bullet points from above hold true.