feat(replay): Calculate hydration diff timestamps based on related hydration breadcrumbs #72561
Conversation
…dration breadcrumbs
github-actions
bot
added
the
Scope: Frontend
Bundle ReportChanges will decrease total bundle size by 681.18kB ⬇️
|
| .getRRWebFrames() | ||
| .findIndex(frame => frame.timestamp < eventTimestampMs); | ||
| const leftFrame = frames.at(Math.max(0, leftReplayFrameIndex)); | ||
| const leftOffsetMs = replayStartTimestamp - (leftFrame?.timestamp ?? 0); |
There was a problem hiding this comment.
am i misunderstanding, or is replayStartTimestamp smaller than leftFrame.timestamp? so this could be negative?
There was a problem hiding this comment.
(or is it being negative expected)
There was a problem hiding this comment.
I set some Math.max() in here, so we'll never get a negative number back.
For the left side, a 0 offset is probably ok to deal with. We could have either an empty html page (unlikely, because we need some html to load the sdk!) or it's just a really early snapshot.
If the right side is 0 then it's a problem for the diff views... I think in that case those views should deal with that themselves... for example we could show an error to the user. 0 in that case might mean that there's no 2nd snapshot to compare against.
| // These stub types should be coming from the sdk, but they're hard-coded until | ||
| // the SDK updates to the latest version... once that happens delete this! | ||
| type StubBreadcrumbTypes = { | ||
| category: 'replay.hydrate-error'; | ||
| timestamp: number; | ||
| type: ''; | ||
| data?: { | ||
| url?: string; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
depends on getsentry/sentry-javascript#12521 being released
|
nice tests👍 |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
michellewzhang
added
the
Trigger: Revert
|
PR reverted: 986296c |
…lated hydration breadcrumbs (#72561)" This reverts commit d29adfe. Co-authored-by: michellewzhang <[email protected]>
…dration breadcrumbs (#73095) This is the 2nd try at #72561 The original was reverted in 986296c because of some errors that popped up: - https://sentry.sentry.io/issues/5505399172/?project=11276&referrer=github-pr-bot - https://sentry.sentry.io/issues/5505401256/?project=11276&referrer=github-pr-bot - https://sentry.sentry.io/issues/5505436186/?project=11276&referrer=github-pr-bot - https://sentry.sentry.io/issues/5505439753/?project=11276&referrer=github-pr-bot - https://sentry.sentry.io/issues/5505459069/?project=11276&referrer=github-pr-bot The difference now is that I've improved the types to include `data.mutations.next`. The real but though was that before, in `breadcrumbItem.tsx`, we were doing the left/right timestamp math only for crumbs that have that mutations.next field... That problem is fixed now because we're checking the crumb type first, then defer to the new `<CrumbHydrationButton>` which will get the offsets and render all at once. Without that fix, we were basically trying to get the left/right offsets for any breadcrumb type, which would easily explode. Related to #70199
3 participants
Related to #70199