Conversation
Signed-off-by: Simo <simo@6529.io>
📝 WalkthroughWalkthroughAdds a client-side drop-open telemetry utility and integrates it into several drop UI flows (calls to startDropOpen). Introduces markDropOpenReady in SingleWaveDropWrapper to record readiness. Removes two badge components (ArtistSubmissionBadge, ProfileWinnerBadge). Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as "UI Component\n(e.g., leaderboard/memes)"
participant Start as "startDropOpen\n(utils)"
participant Registry as "Pending Registry"
participant Sentry
participant Single as "SingleWaveDropWrapper"
participant Mark as "markDropOpenReady\n(utils)"
participant RUM as "AWS RUM"
User->>UI: Click/tap drop
UI->>Start: startDropOpen(dropId, waveId, source, isMobile)
Start->>Registry: store pending entry (start timestamp)
Start->>Sentry: start manual span (optional)
Start-->>UI: return
UI->>UI: invoke onDropClick / navigation
Note right of Single: Drop content mounts / renders
Single->>Mark: markDropOpenReady(dropId, waveId)
Mark->>Registry: lookup pending entry
Registry-->>Mark: return entry (start time)
Mark->>Mark: compute duration_ms (with RAF alignment)
Mark->>Sentry: update span attrs, end span (if present)
Mark->>RUM: recordEvent("drop_popup_ready", payload) (if present)
Mark->>Registry: clear timeout and remove entry
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 84-85: The Sentry packages are mismatched and core is directly
imported in utils/monitoring/dropOpenTiming.ts; either align versions in
package.json (set both "@sentry/core" and "@sentry/nextjs" to the same version,
e.g., ^10.37.0) or remove "@sentry/core" and update the import in
dropOpenTiming.ts to import startSpanManual (and any other APIs) from
"@sentry/nextjs" instead; ensure package.json dependencies reflect the chosen
approach and update any imports that referenced "@sentry/core" to
"@sentry/nextjs" to avoid type/runtime inconsistencies.
In `@utils/monitoring/dropOpenTiming.ts`:
- Around line 30-33: getRoute() currently assumes globalThis.window exists;
guard it and return a safe fallback when window or location is unavailable
(e.g., during SSR or tests). Update getRoute to check for globalThis.window (or
use const win = globalThis.window ?? null) and for win.location before accessing
pathname/search, and return a sensible default (empty string or "/") when
missing; keep the function name getRoute and callers unchanged so locating the
change is trivial.
- Around line 71-73: The timeout callback that currently only calls
pendingOpens.delete(dropId) must also end any Sentry span associated with that
entry to avoid orphaned spans: inside the win.setTimeout handler (the one
creating timeoutId), look up the pending entry from pendingOpens using dropId,
if it contains a Sentry span object (e.g., span or sentrySpan) call
span.finish() (or span.end() if your Sentry wrapper uses that) before deleting
the map entry, and guard the call with a null check so you don't throw if the
span is missing.
🧹 Nitpick comments (1)
utils/monitoring/dropOpenTiming.ts (1)
35-38: Avoid the genericFunctiontype.Using
Functionloses type safety. Consider a more specific signature.Suggested fix
const getRumClient = () => { const win = globalThis.window; - return (win as unknown as { awsRum?: { recordEvent?: Function } }).awsRum; + return (win as unknown as { awsRum?: { recordEvent?: (name: string, data?: Record<string, unknown>) => void } }).awsRum; };
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@utils/monitoring/dropOpenTiming.ts`:
- Around line 38-41: getRumClient currently assumes globalThis.window exists and
accessing win.awsRum will throw in non‑browser/test environments; update
getRumClient to first check that globalThis.window (win) is defined and return
undefined if not, then safely access awsRum (use optional chaining or a null
check) so calls like awsRum?.recordEvent are protected; modify the function
getRumClient to perform the null guard around globalThis.window and return the
awsRum client only when present.
- Around line 65-111: When startDropOpen is invoked, it currently overwrites
pendingOpens[dropId] without cancelling the previous timeout or ending its span;
before creating the new entry, check pendingOpens.get(dropId) and if present
call clearTimeout on its timeoutId and call endSentrySpan on its span (or
otherwise clean it up) to prevent the old timeout from ending the new span, then
proceed to create the new span via startSpanManual and set the new pendingOpens
entry (references: startDropOpen, pendingOpens, timeoutId, endSentrySpan,
startSpanManual, DROP_OPEN_TIMEOUT_MS).
- Around line 113-164: markDropOpenReady's deferred finish closure uses the
captured `entry` but doesn't verify the map still contains the same active entry
after two rAFs, so it can end/delete a newer start for the same dropId; modify
the `finish` function to re-read `pendingOpens.get(entry.dropId)` and confirm
the returned entry is the same object (or has the same `startMs`/waveId) before
proceeding with recording events, ending the span, clearing timeout and deleting
from `pendingOpens`; keep the existing behavior when it matches and simply
return/do nothing if it does not match.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@utils/monitoring/dropOpenTiming.ts`:
- Line 74: The code grabs const win = globalThis.window and later calls
win.setTimeout(...) without verifying win exists; update the logic around the
win variable (and the setTimeout usage) to defensively guard for SSR/test
environments by checking that win is not undefined/null and that win.setTimeout
is a function (or fall back to globalThis.setTimeout or a noop). Locate the
const win = globalThis.window and the call to win.setTimeout(...) and wrap the
timeout invocation with this guard (or use an alternative safe timer) so it
never throws when window is absent.
- Around line 88-106: The span returned by startSpanManual is only active inside
its callback, so stop returning the span for later use; instead, move any
deferred work that needs the active span (like storing it on pendingOpens with
dropId and starting the timeout/handler logic) inside the startSpanManual
callback passed to startSpanManual, or use a different API that keeps the span
active for the needed lifecycle; specifically update the startSpanManual usage
(and avoid assigning span = (createdSpan) => createdSpan) so that
pendingOpens.set(dropId, { span: createdSpan, ... }) and the
timeout/timeout-handler that references pendingOpens.get(dropId)?.span are
executed from within the callback while createdSpan is active.
|
ragnep
approved these changes
Jan 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Removed Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.