Skip to content

fix(logger): stop a server-side jsdom window from silencing all logging in production - #6339

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/logger-jsdom-window-kill-switch
Aug 6, 2026
Merged

fix(logger): stop a server-side jsdom window from silencing all logging in production#6339
waleedlatif1 merged 2 commits into
stagingfrom
fix/logger-jsdom-window-kill-switch

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • shouldLog() treated typeof window !== 'undefined' as "we are in a browser" and, in production, returned false for every level — permanently, process-wide, with no diagnostic
  • ensureDomForTipTap() (apps/sim/lib/collab-doc/converter.ts:68) assigns globalThis.window = jsdomWindow so TipTap can run headless. The first collab-markdown request a task serves therefore kills that task's structured logging for the rest of its life
  • jsdom is a production dependency and next.config.ts explicitly traces it into the standalone output for /api/internal/file-doc/{seed,merge,persist}, so this is live in prod. This is the only non-test window assignment in the repo
  • Fixed by testing for a real browser instead: Node always exposes process.versions.node, a browser never does. The original intent — suppress logging in the browser in production — is preserved

Evidence

This is the cause of the recurring multi-hour structured-logging blackouts:

  • Structured logs went from ~30k/30min to exactly zero while all five ECS tasks kept emitting 400–1,100 raw lines/30min — the processes were alive and console.* worked; only @sim/logger was silent
  • Tasks went dark staggered, one at a time over ~15 minutes — the signature of a per-process latch, not a global event
  • It self-recovered only when five brand-new task IDs appeared (a deploy). A restart cannot fix bad log data; it does clear a latched global
  • Task a5989f51 served file-doc traffic at 15:00 and went dark at 15:08

Type of Change

  • Bug fix

Testing

Two regression tests, both directions:

  • should keep logging when the server installs a DOMred before the fix, green after
  • should stay silent in a real browser — passes both before and after, proving the guard's original behavior is intact and this is not simply removing it

27/27 passing.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 6, 2026 8:31pm

Request Review

@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Small change to logger gating logic with targeted tests; no auth, data, or API surface changes.

Overview
Fixes a process-wide production logging blackout when headless TipTap work installs a jsdom window on the server (ensureDomForTipTap).

Production suppression in shouldLog() no longer treats “window exists” as a browser. A new isBrowserRuntime() helper only silences logs when window is present and process.versions.node is absent, so Node with a fake DOM keeps emitting structured logs while real browsers in production stay quiet.

Regression tests cover both paths: server + jsdom window still logs; browser-like globals with no Node version stay silent.

Reviewed by Cursor Bugbot for commit 05c305c. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR refines production browser detection so a server-side jsdom window no longer disables structured logging while preserving suppression in actual browsers.

  • Adds a Node-runtime discriminator based on process.versions.node.
  • Adds regression coverage for server-installed DOM and browser environments.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/logger/src/index.ts Replaces the window-only production suppression check with runtime-aware browser detection, preserving logging in Node processes that install jsdom.
packages/logger/src/index.test.ts Adds focused regression tests covering both a Node server with an installed DOM and a production browser environment.

Reviews (3): Last reviewed commit: "fix(logger): widen the stubbed process c..." | Re-trigger Greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 05c305c. Configure here.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
waleedlatif1 merged commit 2ab6be6 into staging Aug 6, 2026
5 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/logger-jsdom-window-kill-switch branch August 6, 2026 20:33
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.

1 participant