fix(nextjs): Return correct lastEventId for SSR pages

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run

[s1gr1d]

When using captureUnderscoreErrorException on an _error page, the events were mostly dropped because it already existed from a Sentry-wrapped data fetcher (like getServerProps). This resulted in not sending the error to Sentry but still generating a new event ID which was used as lastEventId (and thus was wrong).

Closes#19217
Also, check out this specific comment within the issue as it gives more context: #19217 (comment)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[andreiborza]

I feel like this has a high chance of not being the event id we are looking for... we really should think about storing lastEventId with more info of what the actual event was that this belongs to.

I don't really think there's much we can do here, just wanted to mention this tho and this is probably better than straight up return an event id of an event that gets deduped.

[logaretm]

I think this can work but doesn't feel bullet proof to me.

Could we perhaps associate error objects with their event IDs? Like add a __sentry_evt_id__ to each error instance so that we can refer to that purely from the error object regardless of where we catch it?

[s1gr1d]

@logaretm that sounds also like a nice approach. But do you mean that independently from lastEventId()? Because I am not sure how those two things would connect. lastEventId() is still separate from an error.

[logaretm]

What I was thinking here is "how do we know for sure that the very last event is the same one that caught the same error?" I can't say I'm familiar with how this all works yet, so I was wondering if this is an edge case to consider, is it possible for events to slip in between? are they guaranteed to be sequential?

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance. Does that track or is it maybe naive?

[s1gr1d]

It can happen that events slip in between (this is also documented in lastEventId):

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

Lines 123 to 136 in c5e3249

	/**
	* 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.
	*/
	exportfunctionlastEventId(): string|undefined{
	returngetIsolationScope().lastEventId();
	}

For example (this was relevant in the case of this PR), captureException is dropping errors that were already recorded:

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

Lines 278 to 285 in b7f8cfe

	publiccaptureException(exception: unknown,hint?: EventHint,scope?: Scope): string{
	consteventId=uuid4();
	// ensure we haven't captured this very object before
	if(checkOrSetAlreadyCaught(exception)){
	DEBUG_BUILD&&debug.log(ALREADY_SEEN_ERROR);
	returneventId;
	}

And this was overriding the event ID.

What I was suggesting is if the error was caught, we stick an event ID on it if possible, that way if it comes up again then we can use the last event assigned to that error instance.

Do you mean that instead of returning a new event ID, we re-use the same one?

[chargome]

This modifies the error object already before sending it, which we do not want bc this prevents it from capturing it?

[logaretm]

probably worth exposing isAlreadyCaptured from core since we want this to be read only.

[github-actions]

Codecov Results 📊

---

Generated by Codecov Action

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	11,060	-	9,100	+22%
GET With Sentry	1,952	18%	1,688	+16%
GET With Sentry (error only)	7,491	68%	6,050	+24%
POST Baseline	1,291	-	1,184	+9%
POST With Sentry	656	51%	575	+14%
POST With Sentry (error only)	1,142	88%	1,020	+12%
MYSQL Baseline	3,524	-	3,137	+12%
MYSQL With Sentry	480	14%	424	+13%
MYSQL With Sentry (error only)	2,996	85%	2,557	+17%

View base workflow run