fix(elysia): Set SDK metadata on the options passed to init - #24521
Shubham-Padkonde wants to merge 4 commits into
Conversation
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>
logaretm
left a comment
There was a problem hiding this comment.
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.
| it('sets SDK metadata on the options passed to initNode', () => { | ||
| init({ dsn: 'https://examplePublicKey@o0.ingest.sentry.io/0' }); | ||
|
|
||
| expect(mockApplySdkMetadata.mock.calls[0]?.[0]).toBe(mockInitNode.mock.calls[0]?.[0]); |
There was a problem hiding this comment.
We avoid asserting via mock.calls[0]?.[0], and I don't think it asserts what we want to test.
Could we drop the applySdkMetadata mock and assert what initNode should receive.
expect(mockInitNode).toHaveBeenCalledWith(
expect.objectContaining({
_metadata: { sdk: expect.objectContaining({ name: 'sentry.javascript.elysia' }) },
}),
);Co-Authored-By: OpenAI Codex <codex@openai.com>
|
Removed the Also added the requested |
Co-Authored-By: OpenAI Codex <codex@openai.com>
logaretm
left a comment
There was a problem hiding this comment.
Tests are failing due to a mockApplySdkMetadata not being defined, probably a simple typo
Co-Authored-By: OpenAI Codex <codex@openai.com>
|
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. |
init()copieduserOptionsintooptionsand then applied SDK metadata touserOptions. 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
initNodereceivessentry.javascript.elysiametadata.Closes #24045
Original contribution generated with Claude Code; review follow-up prepared with OpenAI Codex assistance.