Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(feedback): Replay breadcrumb for feedback events was incorrect #10536

Merged

Conversation

billyvg
Copy link
Member

@billyvg billyvg commented Feb 6, 2024

We are creating a replay breadcrumb when user feedback was submitted, however, the it was not typed correctly, which the timestamp not being included in the proper location.

@AbhiPrasad
Copy link
Member

AbhiPrasad commented Feb 6, 2024

Does this need to go out before v8? I'm planning on a v7 patch release to get out a profiling fix, I can backport these changes onto it too.

@billyvg
Copy link
Member Author

billyvg commented Feb 6, 2024

@AbhiPrasad yeah we could backport it, this is not super high priority though -- when are you aiming for the next release?

@AbhiPrasad
Copy link
Member

yeah we could backport it, this is not super high priority though -- when are you aiming for the next release?

Whenever #10534 can merge and pass CI, hopefully tomorrow

@billyvg
Copy link
Member Author

billyvg commented Feb 6, 2024

yeah we could backport it, this is not super high priority though -- when are you aiming for the next release?

Whenever #10534 can merge and pass CI, hopefully tomorrow

Should I make a PR w/ v7 as base branch?

@AbhiPrasad
Copy link
Member

Should I make a PR w/ v7 as base branch?

Merge it into develop first, we'll cherry-pick it onto v7.

@billyvg billyvg marked this pull request as ready for review February 6, 2024 22:07
@billyvg billyvg requested a review from a team February 6, 2024 22:07
@AbhiPrasad
Copy link
Member

#10534 was merged in, whenever you merge this PR in I'll backport and make a v7 release.

@AbhiPrasad
Copy link
Member

#10542 should fix the CI issues with profiling.

@billyvg
Copy link
Member Author

billyvg commented Feb 7, 2024

[16/285] [chromium] › integrations/httpclient/fetch/withRequest/test.ts:7:11 › works with a Request passed in
  1) [chromium] › integrations/Breadcrumbs/dom/click/test.ts:7:11 › captures Breadcrumb for clicks & debounces them for a second 

    Error: page.evaluate: ReferenceError: Sentry is not defined

        at eval (eval at evaluate (:226:30), <anonymous>:1:1)
        at eval (<anonymous>)
        at UtilityScript.evaluate (<anonymous>:226:30)
        at UtilityScript.<anonymous> (<anonymous>:1:44)
        at eval (/home/runner/work/sentry-javascript/sentry-javascript/dev-packages/browser-integration-tests/eval at evaluate (:226:30), <anonymous>:1:1)
        at UtilityScript.evaluate (/home/runner/work/sentry-javascript/sentry-javascript/dev-packages/browser-integration-tests/<anonymous>:226:30)
        at UtilityScript.<anonymous> (/home/runner/work/sentry-javascript/sentry-javascript/dev-packages/browser-integration-tests/<anonymous>:1:44)
        at /home/runner/work/sentry-javascript/sentry-javascript/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts:37:56

🤔

We are creating a replay breadcrumb when user feedback was submitted, however, the it was not typed correctly, which the timestamp not being included in the proper location.
@mydea mydea force-pushed the fix-feedback-fix-types-and-feedback-breadcrumb-structure branch from 802cc3c to 9e94314 Compare February 7, 2024 08:05
@billyvg billyvg merged commit fa4669a into develop Feb 7, 2024
88 checks passed
@billyvg billyvg deleted the fix-feedback-fix-types-and-feedback-breadcrumb-structure branch February 7, 2024 08:25
mydea pushed a commit that referenced this pull request Feb 7, 2024
…10536)

We are creating a replay breadcrumb when user feedback was submitted,
however, the it was not typed correctly, which the timestamp not being
included in the proper location.
@mydea mydea mentioned this pull request Feb 7, 2024
Sign up for free to 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.

None yet

3 participants