Skip to content

feat: logs proxy using sw#1447

Open
AbhishekChorotiya wants to merge 2 commits intomainfrom
feat/service_worker_logs_proxy
Open

feat: logs proxy using sw#1447
AbhishekChorotiya wants to merge 2 commits intomainfrom
feat/service_worker_logs_proxy

Conversation

@AbhishekChorotiya
Copy link
Copy Markdown
Contributor

@AbhishekChorotiya AbhishekChorotiya commented Mar 26, 2026

fixes: #1446

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Implements a Service Worker-based log proxy mechanism that enables reliable log transmission from the SDK iframe to the logging backend. The Service Worker acts as a middleman that can continue sending logs even when the iframe is unloaded

Problem Statement

  1. Beacon API Limitations: Logs were sent using navigator.sendBeacon() which has reliability issues
  2. Iframe Context Issues: When the payment iframe unloads (e.g., during redirects), pending logs could be lost

Merchant Page Logs (Hyper.res) - Always Beacon API

Merchant Page (Hyper.res) - runs on MERCHANT DOMAIN
         │
         │ 1. Logger initialized with source="hyper_loader"
         │
         ▼
    HyperLogger.make()
         │
         │ 2. Logs collected in mainLogFile array
         │
         ▼
    sendLogsOverNetwork()
         │
         │ 3. ServiceWorkerHelpers.isAvailable()?
         │    → Always FALSE (SW not registered on merchant domain)
         │
         ▼
    fallbackToBeaconApiCall()
         │
         │ 4. beaconApiCall(logs) - sendBeacon to log endpoint
         │
         │ 5. sendCachedLogsFromIDB() - try to send any cached logs
         │
         ▼
    Logs sent via Beacon API

SDK Iframe Logs (Index.res) - Service Worker

SDK Iframe (Index.res)
         │
         │ 1. ServiceWorkerHelpers.registerSW() called on load
         │
         ▼
    Service Worker Registered (/hs-sdk-sw.js)
         │
         │ 2. HyperLogger initialized with source="hyper" or "headless"
         │
         ▼
    User interactions generate logs
         │
         │ 3. Logs collected and batched (20s interval or priority event)
         │
         ▼
    sendLogsOverNetwork()
         │
         │ 4. SW available?
         ▼
    ┌─────────────────┴─────────────────┐
    │ YES                               │ NO
    ▼                                   ▼
postMessage to SW         fallbackToBeaconApiCall()
    │                       (Beacon API + IDB cached)
    ▼                                   │
Service Worker                        ▼
processes SEND_LOGS         POST via sendBeacon
    │                                   │
    │ 5. SW sends new logs              │ 5. Beacon sends logs
    │    + cached IDB logs              │    + IDB cached logs
    ▼                                   ▼
    └──────────────┬────────────────────┘
                   ▼
           POST to GlobalVars.logEndpoint

Bug fixes

1. Race Condition in iDB Retrieve + Clear

Problem: sendCachedLogsFromIDB used separate retrieveLogsFromIndexedDB() and clearLogsFromIndexedDB() calls. Logs added between these operations could be lost or cause duplicates.

Fix: Created retrieveAndClearLogsFromIndexedDB() - atomic operation using single readwrite transaction.

File: src/Utilities/LoggerUtils.res

2. Race Condition in sendLogsToIndexedDB

Problem: Same as above - new logs added during await saveLogsToIndexedDB() were cleared without being saved.

Fix: Same pattern - snapshot + clear before async.

File: src/hyper-log-catcher/HyperLogger.res


How did you test it?

Tested locally

Checklist

  • I ran npm run re:build
  • I reviewed submitted code
  • I added unit tests for my changes where possible

feat: logs proxy using sw

feat: logs proxy using sw
@semanticdiff-com
Copy link
Copy Markdown

semanticdiff-com bot commented Mar 26, 2026

Review changes with  SemanticDiff

Changed Files
File Status
  webpack.common.js  32% smaller
  src/Index.res Unsupported file format
  src/Utilities/LoggerUtils.res Unsupported file format
  src/Window.res Unsupported file format
  src/hyper-log-catcher/ErrorBoundary.res Unsupported file format
  src/hyper-log-catcher/HyperLogger.res Unsupported file format
  src/libraries/IndexedDB.res Unsupported file format
  src/service-worker/ServiceWorker.res Unsupported file format
  src/service-worker/ServiceWorkerHelpers.res Unsupported file format

try {
let bodyStr = logs->JSON.Encode.array->JSON.stringify
let response = await Fetch.fetch(
GlobalVars.logEndpoint,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we take derived endpoint in args and try to keep sw isolated?

@aritro2002
Copy link
Copy Markdown
Contributor

Code Review by opencode

Status: ⚠️ Changes Requested

PR: feat: logs proxy using sw
Branch: feat/service_worker_logs_proxy
Stats: +399 / -152 across 9 files (2 new files)
Build: ✅ Passes (npm run re:build compiles cleanly, no new warnings introduced)


Summary

The PR introduces a Service Worker-based log proxy so that SDK iframe logs can be sent via fetch() from the SW instead of relying solely on navigator.sendBeacon(). It also fixes two race conditions in IndexedDB operations by introducing an atomic retrieveAndClearLogsFromIndexedDB() and snapshotting mainLogFile before clearing.

The architecture is sound: merchant-page logs continue via Beacon API (SW won't be registered on the merchant domain), while iframe logs go through the SW when available, with Beacon API as fallback.


🚨 Critical Issues (Must Fix)

1. GlobalVars.logEndpoint baked into the SW — not overridable at runtime
ServiceWorker.res:11GlobalVars.logEndpoint is a webpack DefinePlugin constant. The SW is a separate webpack entry ("hs-sdk-sw") so DefinePlugin will inject it at build time. However, if the endpoint ever needs to be dynamic (per-merchant or per-env), it can't be overridden at runtime. Consider passing the endpoint via the postMessage payload instead of hardcoding it in the SW. (This aligns with the existing team comment on the PR.)

2. Fire-and-forget dual sends in processMessage may cause duplicate logs
ServiceWorker.res:43-46sendIdbLogs() and sendLogs(newLogs) are both fired with ->ignore and run concurrently. If there's overlap (e.g., some logs were just written to IDB and also appear in the newLogs array), you could get duplicates. Additionally, if sendIdbLogs fails on the fetch, those logs are already cleared from IDB (via retrieveAndClearLogsFromIndexedDB), so they are lost permanently.

3. sendLogs silently swallows failed HTTP responses
ServiceWorker.res:20let _ = response->Fetch.Response.ok checks ok but discards the result. If the POST returns 4xx/5xx, the logs are lost because they've already been cleared from IDB (in the sendIdbLogs path) or removed from the in-memory buffer (in the main thread). Should check response.ok and re-save to IDB on failure.


⚠️ High Priority (Should Fix)

4. Unused functions: retrieveLogsFromIndexedDB, clearLogsFromIndexedDB, saveRawLogsToIndexedDB
LoggerUtils.res:221-286, 335-375 — These functions are defined but never called anywhere in the codebase. retrieveAndClearLogsFromIndexedDB has replaced the separate retrieve+clear pattern. saveRawLogsToIndexedDB is also unused. Remove dead code or explain the intended usage.

5. logFileToObj defines local getStringFromBool instead of using Utils.getStringFromBool
LoggerUtils.res:91 — A local getStringFromBool is defined inside logFileToObj, but Utils.getStringFromBool (at Utils.res:1846) does exactly the same thing. The original code in HyperLogger.res used Utils.getStringFromBool. Per project best practices, use @utils.res utilities instead of rewriting logic.

6. SW registration hardcodes path /hs-sdk-sw.js — scope and path assumptions
ServiceWorkerHelpers.res:24register("/hs-sdk-sw.js") assumes the SW file is served at the root of the iframe's origin. If publicPath is configured to something other than /, the file won't be found. Should derive the path from the webpack publicPath or use a relative path.

7. beforeunload fallback path launches async IDB read that may not complete
HyperLogger.res:142-157 — In the fallback (non-SW) path, sendCachedLogsFromIDB()->ignore launches an async IDB read during beforeunload, which browsers may terminate before completion. The SW path handles this correctly (postMessage is fire-and-forget), but the fallback path has this pre-existing issue.


💡 Medium Priority (Consider Fixing)

8. Console.error in IndexedDB.res:25 has a trailing colon with no variable
IndexedDB.res:25Console.error("IndexedDB is not supported in this environment:") — the trailing colon suggests a variable was intended to follow. Either remove the colon or add context.

9. Commented-out code: ("internal_metadata", ...) in logFileToObj
LoggerUtils.res:117 — Commented-out code should be removed before merge, or tracked as a TODO with an issue reference.


✨ Low Priority (Suggestions)

10. clearLogFile uses a loop with Array.pop — O(n)
HyperLogger.res:120-125 — Consider using a more direct approach like setting length to 0 via a raw JS binding for clarity and performance.

11. Webpack trailing comma formatting changes add noise to the diff
webpack.common.js — Several lines changed only to add trailing commas. Consider separating formatting changes into their own commit.


✅ What's Done Well

  • The retrieveAndClearLogsFromIndexedDB() atomic operation is a solid fix for the race condition — using a single readwrite transaction with getAll + clear is correct.
  • Snapshotting mainLogFile via Array.copy before clearLogFile in sendLogsToIndexedDB and sendLogsOverNetwork correctly prevents the previous race.
  • IndexedDB.getIndexedDBInstance() gracefully handles both window.indexedDB and self.indexedDB contexts, making IndexedDB work in both the main thread and the Service Worker.
  • Good defensive coding with try/catch throughout and proper fallback logic.
  • No Belt usage, no Console.log debug statements, proper ReScript patterns used.

Checklist

  • Build passes (npm run re:build)
  • No Belt modules used
  • No debug logs in production code
  • Dead code removed (unused IDB functions)
  • Utils.getStringFromBool used instead of local reimplementation
  • SW log loss on fetch failure handled

Reviewed by opencode AI using hyperswitch-web skill guidelines

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.

Logs proxy via service worker

3 participants