Skip to content

Conversation

@ericallam
Copy link
Member

@ericallam ericallam commented Jan 7, 2026

Add colored console warnings when the event loop is blocked and wire
a feature flag to enable/disable notifications- Introduce notifyEventLoopBlocked() in eventLoopMonitor.server.ts to
log a colored warning with blocked and async type.

  • Call notifyEventLoopBlocked() when an event-loop stall is detected.
  • Add EVENT_LOOP_MONITOR_NOTIFY_ENABLED to env schema with a default of
    "0" so notifications are off by default.
  • Will notify when over the EVENT_LOOP_MONITOR_THRESHOLD_MS env var

This makes it easier to spot long event-loop stalls during development
or when notifications are explicitly enabled.

CleanShot 2026-01-07 at 15 03 24@2x

Add colored console warnings when the event loop is blocked and wire
a feature flag to enable/disable notifications- Introduce notifyEventLoopBlocked() in eventLoopMonitor.server.ts to
  log a colored warning with blocked and async type.
- Call notifyEventLoopBlocked() when an event-loop stall is detected.
- Add EVENT_LOOP_MONITOR_NOTIFY_ENABLED to env schema with a default of
  "0" so notifications are off by default.

This makes it easier to spot long event-loop stalls during development
or when notifications are explicitly enabled.
@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2026

⚠️ No Changeset found

Latest commit: 1c1d2e7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 7, 2026

Warning

Rate limit exceeded

@ericallam has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 34 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 062766e and 1c1d2e7.

📒 Files selected for processing (2)
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/eventLoopMonitor.server.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Jan 7, 2026

PR Review: feat: add event loop block notifications and env flag

Thanks for this PR! Adding colored console warnings for event loop blocking is a nice debugging enhancement. Here's my review:

✅ What looks good

  1. Follows existing patterns: The env variable naming (EVENT_LOOP_MONITOR_NOTIFY_ENABLED) and string-based boolean pattern (z.string().default("0")) is consistent with dozens of other feature flags in env.server.ts

  2. Safe by default: Notifications are disabled by default ("0"), which is appropriate for production environments

  3. Minimal and focused change: The PR is small and well-scoped to a single concern

💡 Suggestions for improvement

  1. Consider using the structured logger instead of console.warn
    The codebase has a structured logger at apps/webapp/app/services/logger.server.ts that integrates with Sentry and provides consistent log formatting. While console.warn with ANSI colors is more visually prominent (which seems intentional for dev debugging), you could consider:

    logger.warn("Event loop blocked", { timeMs, asyncType });

    This would give you structured logging with automatic Sentry integration. However, if the goal is specifically to have eye-catching colored terminal output for local debugging, the current approach is reasonable.

  2. Minor: ANSI color compatibility
    The ANSI codes will work great in most terminals, but might produce garbled output in environments that don't support them (e.g., some CI logs, Windows terminals, log aggregators). Consider:

    • Adding a check for process.stdout.isTTY if this becomes an issue
    • Or document that this feature is intended for local development only
  3. Consider adding a CYAN or similar for the async type
    Currently the async type is displayed without color highlighting. For consistency with the rest of the message, you might want to highlight it as well.

🔍 Code quality observations

  • Clean implementation: The notifyEventLoopBlocked function is well-isolated and has early return for the disabled case
  • Correct placement: Calling notifyEventLoopBlocked after newSpan.end() ensures the telemetry is sent even if the console logging fails

❓ Questions

  1. Is there a reason to prefer console.warn over the structured logger for this use case? If it's to ensure visibility during local development, that makes sense - just curious about the intent.

Summary

This is a solid, low-risk enhancement. The implementation follows existing patterns and is disabled by default. Consider the logger vs console.warn tradeoff, but the current approach is acceptable for a debugging feature meant to be eye-catching.

Recommendation: ✅ Approve with optional suggestions

@matt-aitken matt-aitken merged commit 09460ab into main Jan 7, 2026
30 checks passed
@matt-aitken matt-aitken deleted the ea-branch-113-4 branch January 7, 2026 15:04
@vibe-kanban-cloud
Copy link

Review Complete

Your review story is ready!

View Story

Comment !reviewfast on this PR to re-generate the story.

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.

3 participants