Skip to content

feat(v8/bun): Update @sentry/bun to use OTEL node#10997

Merged
AbhiPrasad merged 13 commits intodevelopfrom
abhi-bun-sentry-node
Mar 13, 2024
Merged

feat(v8/bun): Update @sentry/bun to use OTEL node#10997
AbhiPrasad merged 13 commits intodevelopfrom
abhi-bun-sentry-node

Conversation

@AbhiPrasad
Copy link
Copy Markdown
Contributor

@AbhiPrasad AbhiPrasad commented Mar 8, 2024

ref #9956

Given Bun is just a wrapper over Node, we should be good to go with just these changes.

@AbhiPrasad AbhiPrasad requested a review from a team March 8, 2024 19:40
@AbhiPrasad AbhiPrasad self-assigned this Mar 8, 2024
@AbhiPrasad AbhiPrasad requested review from s1gr1d and stephanie-anderson and removed request for a team March 8, 2024 19:40
@AbhiPrasad AbhiPrasad mentioned this pull request Mar 8, 2024
Closed
@AbhiPrasad
Copy link
Copy Markdown
Contributor Author

AbhiPrasad commented Mar 8, 2024 

yarn run v1.22.21
$ pnpm test

> node-express-app@1.0.0 test /home/runner/work/sentry-javascript/sentry-javascript/dev-packages/e2e-tests/test-applications/node-exports-test-app
>  bun run ./dist/consistentExports.js

error: Cannot find module "@sentry/bun" from "/home/runner/work/sentry-javascript/sentry-javascript/dev-packages/e2e-tests/test-applications/node-exports-test-app/dist/consistentExports.js"
 ELIFECYCLE  Test failed. See above for more details.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

Very confused why this error is happening. Cache issue? 🤔

@timfish
Copy link
Copy Markdown
Collaborator

timfish commented Mar 11, 2024

Run a yarn clean && yarn build:dev and take a look at the output directories?

I've had this a few times recently and for me it was caused by a Rollup bug where preserveModules + a module not marked as external causing the output directory structure to be completely wrong.

@AbhiPrasad
Copy link
Copy Markdown
Contributor Author

AbhiPrasad commented Mar 11, 2024

Well TIL about this

sentry-javascript/packages/bun/rollup.npm.config.mjs

Lines 5 to 6 in 0caddec

// remove cjs from config array config[0].output.format == cjs
export default [config[1]];

I assume that is what is causing issues.

@AbhiPrasad AbhiPrasad force-pushed the abhi-bun-sentry-node branch from 94b8789 to 815832c Compare March 11, 2024 20:03
@AbhiPrasad AbhiPrasad mentioned this pull request Mar 12, 2024
Merged
@AbhiPrasad AbhiPrasad force-pushed the abhi-bun-sentry-node branch from 6c74347 to e8fc0cf Compare March 12, 2024 17:15
AbhiPrasad
AbhiPrasad commented Mar 12, 2024
View reviewed changes
dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts Outdated
import * as SentrySvelteKit from '@sentry/sveltekit';

// Using CJS export for Bun SDK
const SentryBun = require('@sentry/bun');
Copy link
Copy Markdown
Contributor Author

@AbhiPrasad AbhiPrasad Mar 12, 2024

Choose a reason for hiding this comment

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

Not sure why CI is failing, trying this to see if it helps.

Copy link
Copy Markdown
Collaborator

@timfish timfish Mar 12, 2024

Choose a reason for hiding this comment

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

This was timing out in CI for me yesterday. I tried it locally and it passed and then tried for a 4th time in CI and it passed there too 🤔

@AbhiPrasad
Copy link
Copy Markdown
Contributor Author

AbhiPrasad commented Mar 12, 2024

Found the bug! I was accidently importing node-experimental 🤦

@AbhiPrasad AbhiPrasad force-pushed the abhi-bun-sentry-node branch from 662a7e6 to 2493d65 Compare March 12, 2024 18:44
@AbhiPrasad AbhiPrasad merged commit 7879791 into develop Mar 13, 2024
@AbhiPrasad AbhiPrasad deleted the abhi-bun-sentry-node branch March 13, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timfish timfish timfish left review comments

@mydea mydea mydea approved these changes

@stephanie-anderson stephanie-anderson Awaiting requested review from stephanie-anderson stephanie-anderson was automatically assigned from getsentry/team-javascript-sdks

@s1gr1d s1gr1d Awaiting requested review from s1gr1d s1gr1d was automatically assigned from getsentry/team-javascript-sdks

Assignees

@AbhiPrasad AbhiPrasad

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AbhiPrasad @timfish @mydea