refactor: improve video tracking #1682
Conversation
|
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new mechanism was added to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VideoTracker
participant Navigator
participant Server
User->>VideoTracker: Changes visibility (hidden/visible) or triggers unload
alt Visibility changes to hidden and not tracked
VideoTracker->>VideoTracker: Set hasTrackedUnload = true
alt Video is playing
VideoTracker->>VideoTracker: trackEvent("played", useBeacon=true)
VideoTracker->>Navigator: sendBeacon or fetch with keepalive
Navigator->>Server: Deliver "played" event
end
VideoTracker->>VideoTracker: trackEvent("blur", useBeacon=true)
VideoTracker->>Navigator: sendBeacon or fetch with keepalive
Navigator->>Server: Deliver "blur" event
else Visibility changes to visible
VideoTracker->>VideoTracker: Reset hasTrackedUnload = false
VideoTracker->>VideoTracker: trackEvent("focus")
VideoTracker->>Navigator: fetch (normal)
Navigator->>Server: Deliver "focus" event
end
alt cleanup called and video playing and not tracked
VideoTracker->>VideoTracker: trackEvent("played", useBeacon=true)
VideoTracker->>Navigator: sendBeacon or fetch with keepalive
Navigator->>Server: Deliver "played" event
end
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/tracking/video-tracking.ts (1)
81-86: Consider handling promise rejections from the async trackEvent method.The
trackEventmethod is now async, but the debounced handlers don't await or handle potential rejections. While fire-and-forget might be intentional for tracking, unhandled promise rejections could cause issues.Consider wrapping the async call with error handling:
const createDebouncedHandler = (wait: number, immediate = false) => debounce( (eventType: EventType, endTime?: number) => { console.log( `[VideoTracker] Tracking event: ${eventType} - ${endTime}`, ); - return this.trackEvent(eventType, endTime); + return this.trackEvent(eventType, endTime).catch((error) => { + console.error(`[VideoTracker] Failed to track event ${eventType}:`, error); + }); }, wait, immediate, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/tracking/video-tracking.ts(4 hunks)
🔇 Additional comments (3)
lib/tracking/video-tracking.ts (3)
63-63: LGTM!The
hasTrackedUnloadflag is properly initialized and will help prevent duplicate unload event tracking.
341-352: LGTM!The visibility change tracking logic is well-implemented. It properly uses the beacon API for reliability during page hide events and prevents duplicate tracking with the
hasTrackedUnloadflag.
361-364: LGTM!The cleanup logic correctly tracks the final "played" event using the beacon API and prevents duplicate tracking with the
hasTrackedUnloadflag.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Summary by CodeRabbit