Skip to content

fix(core): Do not overwrite user provided conversation id in Vercel - #19903

Merged
nicohrubec merged 5 commits into
developfrom
nh/openai-conditional-conversation-id
Mar 20, 2026
Merged

fix(core): Do not overwrite user provided conversation id in Vercel#19903
nicohrubec merged 5 commits into
developfrom
nh/openai-conditional-conversation-id

Conversation

@nicohrubec

@nicohrubecnicohrubec commented Mar 20, 2026

Copy link
Copy Markdown
Member

We set the conversation id unconditionally based on the resonse_id, so if a user calls Sentry.setConversationId() explicitly this will still be overwritten, which is unexpected.

Closes#19904 (added automatically)

@github-actions

github-actionsBot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

Deps

  • Bump mongodb-memory-server-global from 10.1.4 to 11.0.1 by dependabot in #19888
  • Bump stacktrace-parser from 0.1.10 to 0.1.11 by dependabot in #19887

Bug Fixes 🐛

Core

  • Do not overwrite user provided conversation id in Vercel by nicohrubec in #19903
  • Return same value from startSpan as callback returns by s1gr1d in #19300

Other

  • (cloudflare) Forward ctx argument to Workflow.do user callback by Lms24 in #19891
  • (deps) Bump socket.io-parser to 4.2.6 to fix CVE-2026-33151 by chargome in #19880
  • (nestjs) Add node to nest metadata by chargome in #19875
  • (serverless) Add node to metadata by nicohrubec in #19878

Internal Changes 🔧

  • (astro) Re-enable server island tracing e2e test in Astro 6 by Lms24 in #19872
  • (lint) Resolve oxlint warnings by isaacs in #19893
  • (node-integration-tests) Remove unnecessary file-type dependency by Lms24 in #19824

🤖 This preview updates automatically when you update the PR.

@nicohrubecnicohrubec changed the title fix(core): Do not overwrite conversation id if already setfix(core): Do not overwrite conversation id if already set in VercelMar 20, 2026
@github-actions

github-actionsBot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

⚠️Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

PathSize% ChangeChange
@sentry/browser25.69 kB+0.2%+49 B 🔺
@sentry/browser - with treeshaking flags24.17 kB+0.14%+33 B 🔺
@sentry/browser (incl. Tracing)42.67 kB+0.13%+54 B 🔺
@sentry/browser (incl. Tracing, Profiling)47.33 kB+0.12%+55 B 🔺
@sentry/browser (incl. Tracing, Replay)81.48 kB+0.08%+57 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags71.06 kB+0.1%+69 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas)86.17 kB+0.06%+50 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback)98.41 kB+0.04%+36 B 🔺
@sentry/browser (incl. Feedback)42.48 kB+0.08%+30 B 🔺
@sentry/browser (incl. sendFeedback)30.35 kB+0.15%+43 B 🔺
@sentry/browser (incl. FeedbackAsync)35.4 kB+0.12%+39 B 🔺
@sentry/browser (incl. Metrics)26.96 kB+0.15%+38 B 🔺
@sentry/browser (incl. Logs)27.1 kB+0.12%+32 B 🔺
@sentry/browser (incl. Metrics & Logs)27.78 kB+0.15%+39 B 🔺
@sentry/react27.45 kB+0.22%+58 B 🔺
@sentry/react (incl. Tracing)45.01 kB+0.14%+60 B 🔺
@sentry/vue30.13 kB+0.16%+46 B 🔺
@sentry/vue (incl. Tracing)44.52 kB+0.09%+39 B 🔺
@sentry/svelte25.7 kB+0.16%+40 B 🔺
CDN Bundle28.35 kB+0.27%+75 B 🔺
CDN Bundle (incl. Tracing)43.57 kB+0.15%+62 B 🔺
CDN Bundle (incl. Logs, Metrics)29.22 kB+0.27%+77 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics)44.43 kB+0.17%+75 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics)68.29 kB+0.13%+85 B 🔺
CDN Bundle (incl. Tracing, Replay)80.41 kB+0.1%+73 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)81.31 kB+0.1%+76 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback)85.97 kB+0.12%+103 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)86.86 kB+0.1%+86 B 🔺
CDN Bundle - uncompressed82.7 kB+0.1%+77 B 🔺
CDN Bundle (incl. Tracing) - uncompressed128.62 kB+0.05%+64 B 🔺
CDN Bundle (incl. Logs, Metrics) - uncompressed85.57 kB+0.1%+77 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed131.49 kB+0.05%+64 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed209.22 kB+0.05%+102 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed245.5 kB+0.04%+89 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed248.35 kB+0.04%+89 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed258.41 kB+0.04%+89 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed261.26 kB+0.04%+89 B 🔺
@sentry/nextjs (client)47.4 kB+0.08%+37 B 🔺
@sentry/sveltekit (client)43.12 kB+0.12%+51 B 🔺
@sentry/node-core56.42 kB+0.13%+73 B 🔺
@sentry/node173.38 kB+0.13%+221 B 🔺
@sentry/node - without tracing96.43 kB+0.1%+87 B 🔺
@sentry/aws-serverless113.44 kB+0.09%+100 B 🔺

View base workflow run

@github-actions

github-actionsBot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

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.
⚠️Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

ScenarioRequests/s% of BaselinePrev. Requests/sChange %
GET Baseline8,622-9,006-4%
GET With Sentry1,64619%1,671-1%
GET With Sentry (error only)6,06270%6,054+0%
POST Baseline1,210-1,177+3%
POST With Sentry58348%577+1%
POST With Sentry (error only)1,05087%1,018+3%
MYSQL Baseline3,212-3,139+2%
MYSQL With Sentry41313%459-10%
MYSQL With Sentry (error only)2,62082%2,578+2%

View base workflow run

@Lms24Lms24 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change LGTM but I agree with bugbot that we should harden the test.

L: Not sure if this is a me-Problem but I had a hard time understanding the PR title. IIUC this fix ensures we don't overwrite a user-provided (custom) conversationId. Maybe we can change the title to something like "do not overwrite user-provided conversation id"? Just a suggestion, feel free to disregard

@chargome

Copy link
Copy Markdown
Member

The change itself looks fine to me, but improving the test sounds good 👍

@nicohrubecnicohrubec changed the title fix(core): Do not overwrite conversation id if already set in Vercelfix(core): Do not overwrite user provided conversation id in VercelMar 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@nicohrubec

Copy link
Copy Markdown
MemberAuthor

@Lms24 thanks for the suggestions I updated the test to be more strict and I agree that your proposed PR title should be a bit more clear so I changed it as well

@nicohrubec
nicohrubec merged commit 587cc6c into developMar 20, 2026
236 checks passed
@nicohrubec
nicohrubec deleted the nh/openai-conditional-conversation-id branch March 20, 2026 11:43
s1gr1d pushed a commit that referenced this pull request Mar 20, 2026
…19903)
We set the conversation id unconditionally based on the `resonse_id`, so
if a user calls `Sentry.setConversationId()` explicitly this will
be overwritten, which is unexpected.
Closes#19904 (added automatically)
s1gr1d pushed a commit that referenced this pull request Mar 20, 2026
…19903)
We set the conversation id unconditionally based on the `resonse_id`, so
if a user calls `Sentry.setConversationId()` explicitly this will
be overwritten, which is unexpected.
Closes#19904 (added automatically)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(core): Do not overwrite user provided conversation id in Vercel

3 participants

@nicohrubec@chargome@Lms24