Skip to content

fix(elysia): Set SDK metadata on the options passed to init - #24521

Merged
isaacs merged 5 commits into
getsentry:developfrom
Shubham-Padkonde:fix/elysia-sdk-metadata
Oct 18, 2026
Merged

fix(elysia): Set SDK metadata on the options passed to init#24521
isaacs merged 5 commits into
getsentry:developfrom
Shubham-Padkonde:fix/elysia-sdk-metadata

Conversation

@Shubham-Padkonde

@Shubham-Padkonde Shubham-Padkonde commented Sep 19, 2026 •
edited by isaacs
Loading

Copy link
Copy Markdown
Contributor

init() copied userOptions into options and then applied SDK metadata to userOptions. When metadata was initially absent, the copied options passed to the underlying SDK did not receive Elysia's SDK name.

Apply SDK metadata to the options passed to the underlying SDK. The regression test uses the real metadata helper and checks that initNode receives sentry.javascript.elysia metadata.

Fixes #24045
Fixes JS-3580

@Shubham-Padkonde
Shubham-Padkonde requested a review from a team as a code owner September 19, 2026 09:30
@Shubham-Padkonde
Shubham-Padkonde requested review from mydea and s1gr1d and removed request for a team September 19, 2026 09:30
@github-actions

github-actions Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

👋 @mydea, @s1gr1d — Please review this PR when you get a chance!

@s1gr1d
s1gr1d requested a review from logaretm September 24, 2026 12:21
logaretm
logaretm requested changes Sep 24, 2026
View reviewed changes

@logaretm logaretm 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.

Thanks for the PR, it looks good mostly but I think we need to test this properly. I've called out a simple change in the existing test.

We should also assert the SDK metadata in both e2e test Elysia apps we have, each should detect the runtime/name correctly.

You can add an assertions in these test cases:

Something like this should be fine:

expect(errorEvent.sdk?.name).toBe('sentry.javascript.elysia');

That checks the real event on both runtimes.

Comment thread packages/elysia/test/sdk.test.ts Outdated
@Shubham-Padkonde

Shubham-Padkonde commented Sep 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Removed the applySdkMetadata mock and now assert the actual Elysia metadata received by initNode, as requested. Confirmed that the revised test fails when metadata is applied to userOptions instead of options, and all 31 Elysia package tests pass with the fix. Formatting and lint checks pass on the changed unit test.

Also added the requested errorEvent.sdk?.name assertion to both the Node and Bun E2E error tests. Their formatting checks pass. Local E2E execution stopped before running tests because this checkout has no packed SDK tarballs; those two runtime checks still need CI verification. I did not run the full monorepo suite. Prepared with OpenAI Codex assistance.

logaretm
logaretm reviewed Sep 24, 2026
View reviewed changes

@logaretm logaretm 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.

Tests are failing due to a mockApplySdkMetadata not being defined, probably a simple typo

@Shubham-Padkonde

Shubham-Padkonde commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

The failure came from the new Bun-runtime test added on develop: the automatic CI merge retained its reference to the removed mock. Merged develop and changed that assertion to verify the runtime and actual Elysia SDK metadata received by initNode (7fe29ad).

All 32 Elysia package tests now pass locally, including the Bun-runtime case; the changed test passes formatting and lint. The E2E Hydrogen failures in the previous run are separate from this missing-reference failure and are not claimed fixed by this change. Prepared with Codex assistance.

@s1gr1d s1gr1d added the external PR from an external contributor label Sep 29, 2026
@isaacs isaacs mentioned this pull request Oct 18, 2026
Closed
isaacs
isaacs approved these changes Oct 18, 2026
View reviewed changes

@isaacs isaacs left a comment •
edited
Loading

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.

Updated to land the remaining changes @logaretm suggested. Will land once CI passes and it gets the 👍

Shubham-Padkonde and others added 4 commits October 17, 2026 18:43
@Shubham-Padkonde @claude @isaacs
fix(elysia): Set SDK metadata on the options passed to init
e4ae88d 
applySdkMetadata was applied to userOptions after they had already been
copied into the options passed to @sentry/bun's init, so the copy kept no
metadata and Bun set its own. Events were reported as
sentry.javascript.bun instead of sentry.javascript.elysia.

Fixes getsentry#24045

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Shubham-Padkonde @codex @isaacs
test(elysia): assert metadata received by the runtime SDK
ba5af5a 
Co-Authored-By: OpenAI Codex <codex@openai.com>
@Shubham-Padkonde @codex @isaacs
test(elysia): assert SDK name in both runtime apps
9fc226a 
Co-Authored-By: OpenAI Codex <codex@openai.com>
@isaacs @claude
test(elysia): Assert SDK packages per runtime
d5e0a0d 
Check that the SDK metadata lists `@sentry/elysia` and the runtime
package (`@sentry/node` or `@sentry/bun`). Assert the detected Node
runtime with `toHaveBeenCalledWith` instead of reading `mock.calls`.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@isaacs
isaacs force-pushed the fix/elysia-sdk-metadata branch from a6565d3 to d5e0a0d Compare October 18, 2026 01:45
@isaacs
isaacs enabled auto-merge (squash) October 18, 2026 01:46
@isaacs
isaacs requested a review from logaretm October 18, 2026 01:46
@isaacs isaacs mentioned this pull request Oct 18, 2026
Merged
@isaacs
isaacs disabled auto-merge October 18, 2026 02:03
@isaacs
isaacs enabled auto-merge (squash) October 18, 2026 03:03
@isaacs
isaacs merged commit 6244a8b into getsentry:develop Oct 18, 2026
42 checks passed
@javascript-sdk-gitflow javascript-sdk-gitflow Bot mentioned this pull request Oct 18, 2026
Open
@sentry-junior sentry-junior Bot mentioned this pull request Oct 18, 2026
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@isaacs isaacs isaacs approved these changes
@logaretm logaretm logaretm approved these changes
@mydea mydea Awaiting requested review from mydea mydea is a code owner automatically assigned from getsentry/team-javascript-sdks-framework
@s1gr1d s1gr1d Awaiting requested review from s1gr1d s1gr1d is a code owner automatically assigned from getsentry/team-javascript-sdks-framework

Assignees

No one assigned

Labels

external PR from an external contributor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Elysia events report as sentry.javascript.bun

4 participants

@Shubham-Padkonde @isaacs @logaretm @s1gr1d