Skip to content

Conversation

gggritso
Copy link
Member

@gggritso gggritso commented Nov 1, 2024

See comment in details. Prevents errors and transaction events with names like "//dashboards/:dashboardId" caused by a bad interaction between React Router 6 instrumentation in the SDK and our shimming setup.

I'm not sure if other kinds of telemetry (Profiles, Replays, breadcrumbs) also need a fix, but it didn't seem like it.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Nov 1, 2024
@getsantry

This comment was marked as outdated.

@getsantry getsantry bot added Stale and removed Stale labels Nov 22, 2024
@getsantry getsantry bot added the Stale label Dec 15, 2024
@getsantry

This comment was marked as outdated.

@getsantry getsantry bot removed the Stale label Dec 16, 2024
@gggritso gggritso force-pushed the fix/prevent-double-leading-slashes-in-transaction-names branch from 1a7aed1 to 16f6e8a Compare December 27, 2024 17:05
Copy link

codecov bot commented Dec 27, 2024

❌ 7 Tests Failed:

Tests completed Failed Passed Skipped
8438 7 8431 0
View the top 3 failed tests by shortest run time
WidgetBuilder Events Widgets Custom Performance Metrics can choose a custom measurement
Stack Traces | 3.07s run time
TypeError: Cannot read properties of null (reading 'parentNode')
    at getReactSelectContainerFromInput (.../js/sentry-test/selectEvent.tsx:39:38)
    at Object.select (.../js/sentry-test/selectEvent.tsx:99:19)
    at Object.<anonymous> (.../dashboards/widgetBuilder/widgetBuilderDataset.spec.tsx:704:9)
InviteRowControlNew updates email addresses when new emails are inputted invokes the mock correctly with many using delimiter ","
Stack Traces | 5.02s run time
Error: thrown: "Exceeded timeout of 5000 ms for a test.
Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."
    at .../modals/inviteMembersModal/inviteRowControlNew.spec.tsx:80:5
    at .../jest-each/build/bind.js:81:13
    at _dispatchDescribe (.../jest-circus/build/index.js:91:26)
    at describe (.../jest-circus/build/index.js:55:5)
    at .../jest-each/build/bind.js:47:15
    at Array.forEach (<anonymous>)
    at eachBind (.../jest-each/build/bind.js:39:22)
    at .../modals/inviteMembersModal/inviteRowControlNew.spec.tsx:64:6
    at _dispatchDescribe (.../jest-circus/build/index.js:91:26)
    at describe (.../jest-circus/build/index.js:55:5)
    at Object.<anonymous> (.../modals/inviteMembersModal/inviteRowControlNew.spec.tsx:10:1)
    at Runtime._execModule (.../jest-runtime/build/index.js:1439:24)
    at Runtime._loadModule (.../jest-runtime/build/index.js:1022:12)
    at Runtime.requireModule (.../jest-runtime/build/index.js:882:12)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:77:13)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
InviteRowControlNew updates email addresses when new emails are inputted invokes the mock correctly with many using delimiter "{enter}"
Stack Traces | 5.04s run time
Error: thrown: "Exceeded timeout of 5000 ms for a test.
Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."
    at .../modals/inviteMembersModal/inviteRowControlNew.spec.tsx:80:5
    at .../jest-each/build/bind.js:81:13
    at _dispatchDescribe (.../jest-circus/build/index.js:91:26)
    at describe (.../jest-circus/build/index.js:55:5)
    at .../jest-each/build/bind.js:47:15
    at Array.forEach (<anonymous>)
    at eachBind (.../jest-each/build/bind.js:39:22)
    at .../modals/inviteMembersModal/inviteRowControlNew.spec.tsx:64:6
    at _dispatchDescribe (.../jest-circus/build/index.js:91:26)
    at describe (.../jest-circus/build/index.js:55:5)
    at Object.<anonymous> (.../modals/inviteMembersModal/inviteRowControlNew.spec.tsx:10:1)
    at Runtime._execModule (.../jest-runtime/build/index.js:1439:24)
    at Runtime._loadModule (.../jest-runtime/build/index.js:1022:12)
    at Runtime.requireModule (.../jest-runtime/build/index.js:882:12)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:77:13)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@gggritso gggritso marked this pull request as ready for review December 27, 2024 17:23
@gggritso gggritso requested a review from a team December 27, 2024 17:23
Copy link
Member

@malwilley malwilley left a comment

Choose a reason for hiding this comment

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

This PR should fix this for real: getsentry/sentry-javascript#14821

But this seems like a good interim fix

@gggritso
Copy link
Member Author

@malwilley oh nice, I didn't see that! Since that PR's the proper fix, and this has already been broken for an eternity, I don't think this interim fix is really worthwhile, I'm just gonna close it 👍🏻

@gggritso gggritso closed this Dec 27, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants