Skip to content

feat: capture uncaught IPC handler errors as PostHog exceptions#2886

Merged
wwwillchen merged 1 commit intodyad-sh:mainfrom
wwwillchen:capture-ipc-handler-exceptions-posthog
Mar 5, 2026
Merged

feat: capture uncaught IPC handler errors as PostHog exceptions#2886
wwwillchen merged 1 commit intodyad-sh:mainfrom
wwwillchen:capture-ipc-handler-exceptions-posthog

Conversation

@wwwillchen
Copy link
Copy Markdown
Collaborator

@wwwillchen wwwillchen commented Mar 4, 2026

Summary

  • Added sendTelemetryException() utility that sends $exception events to PostHog via the main→renderer telemetry bridge, including error type, message, and stack trace
  • Wired error capture into all three IPC handler factories (createTypedHandler, createLoggedTypedHandler, createLoggedHandler) so every ipcMain.handle error is automatically reported with the IPC channel name as context
  • Errors are still re-thrown after capture so existing behavior is unchanged

Test plan

  • All 844 existing tests pass
  • Verify in dev that IPC handler errors appear as $exception events in PostHog with ipc_channel property
  • Confirm telemetry opt-out still prevents exception events from being sent (via existing before_send filter)

🤖 Generated with Claude Code


Open with Devin

Add sendTelemetryException() utility that sends $exception events to
PostHog via the renderer bridge. Wire it into all three IPC handler
factories (createTypedHandler, createLoggedTypedHandler,
createLoggedHandler) so every ipcMain.handle error is reported with
the channel name as context.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wwwillchen
Copy link
Copy Markdown
Collaborator Author

@BugBot run

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the application's error monitoring capabilities by implementing a robust system for capturing uncaught Inter-Process Communication (IPC) handler errors. It introduces a new telemetry utility to report these exceptions to PostHog, providing valuable insights into runtime issues without altering the application's existing error propagation.

Highlights

  • New Telemetry Utility: Introduced sendTelemetryException() to send $exception events to PostHog, including error type, message, and stack trace, via the main→renderer telemetry bridge.
  • IPC Error Capture: Integrated error capture into createTypedHandler, createLoggedTypedHandler, and createLoggedHandler IPC factories, ensuring all ipcMain.handle errors are automatically reported with the IPC channel name.
  • Behavior Preservation: Ensured that errors are re-thrown after telemetry capture, maintaining existing error handling behavior.
Changelog
  • src/ipc/handlers/base.ts
    • Imported sendTelemetryException utility.
    • Wrapped handler execution in createTypedHandler with a try-catch block to send exceptions to telemetry.
    • Added sendTelemetryException call within the existing catch block of createLoggedTypedHandler.
  • src/ipc/handlers/safe_handle.ts
    • Imported sendTelemetryException utility.
    • Added sendTelemetryException call within the existing catch block of createLoggedHandler.
  • src/ipc/utils/telemetry.ts
    • Added sendTelemetryException function to format and send error details as a PostHog $exception event.
Activity
  • No specific activity (comments, reviews, progress) has been recorded for this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR adds automatic PostHog exception telemetry for all uncaught IPC handler errors by introducing a sendTelemetryException utility and wiring it into all three IPC handler factories (createTypedHandler, createLoggedTypedHandler, createLoggedHandler). Errors are still re-thrown after capture, so existing error-handling behaviour is completely preserved.

  • src/ipc/utils/telemetry.ts: New sendTelemetryException helper normalises unknown errors, extracts name, message, and stack, then forwards a $exception event via the existing main→renderer PostHog bridge.
  • src/ipc/handlers/base.ts: A try/catch block wraps the handler call in createTypedHandler; createLoggedTypedHandler's existing catch block now additionally calls sendTelemetryException. Input-validation errors are intentionally not reported.
  • src/ipc/handlers/safe_handle.ts: sendTelemetryException is called with the original error in createLoggedHandler before the error is wrapped and re-thrown.

Confidence Score: 5/5

  • Safe to merge — errors are re-thrown unchanged, the telemetry call is fire-and-forget, and all 844 tests pass.
  • Changes are additive and non-breaking: the telemetry call is wrapped in try/catch internally, errors are always re-thrown unchanged, existing error-handling behaviour is preserved, and all tests pass. The implementation correctly captures the original error before any wrapping occurs.
  • No files require special attention

Sequence Diagram

sequenceDiagram
    participant R as Renderer
    participant IPC as ipcMain handler<br/>(createTypedHandler /<br/>createLoggedTypedHandler /<br/>createLoggedHandler)
    participant H as Business Logic Handler
    participant T as sendTelemetryException
    participant TE as sendTelemetryEvent
    participant BW as BrowserWindow

    R->>IPC: invoke(channel, args)
    IPC->>H: handler(event, parsed.data)
    H-->>IPC: throws Error
    IPC->>T: sendTelemetryException(err, { ipc_channel })
    T->>TE: sendTelemetryEvent("$exception", { $exception_type, $exception_message, $exception_stack_trace_raw, ipc_channel })
    TE->>BW: webContents.send("telemetry:event", payload)
    BW-->>R: PostHog captures $exception event
    IPC-->>R: re-throw original error
Loading

Last reviewed commit: 19beed1

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown

@cursor cursor bot left a comment

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 4, 2026

🔍 Dyadbot Code Review Summary

Verdict: ✅ YES - Ready to merge

Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard.

✅ No actionable issues found. This is a clean, focused PR that adds telemetry exception capture to all three IPC handler factories without changing existing behavior.

What we checked:

  • Error re-throw semantics are preserved (errors are still re-thrown after telemetry capture)
  • sendTelemetryException correctly normalizes non-Error thrown values
  • Telemetry is wired into all three handler factories consistently
  • No new security concerns (telemetry flows through existing PostHog bridge with consent gating)
🚫 Dropped False Positives (4 items)
  • Error wrapping loses original stack in safe_handle.ts (all 3 agents flagged) — Dropped: Pre-existing pattern in createLoggedHandler that wraps errors via throw new Error(...). This PR didn't introduce it, and the telemetry correctly captures the original error before the wrap.
  • Output validation errors misattributed in createLoggedTypedHandler — Dropped: safeParse never throws; it returns a result object. The output validation code only logs via console.error, so it cannot be caught by the handler's catch block.
  • Telemetry silently dropped when no renderer window is open (2 agents flagged) — Dropped: Pre-existing limitation of the existing sendTelemetryEvent infrastructure, not introduced by this PR.
  • PostHog listener timing edge case at startup — Dropped: Pre-existing architecture concern about onTelemetryEvent listener registration timing, unrelated to this change.

Generated by Dyadbot multi-agent code review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19beed138b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files

Confidence score: 3/5

  • There is a concrete medium-severity issue in src/ipc/utils/telemetry.ts: placing ...context after $exception_* fields allows context keys to overwrite exception metadata silently.
  • With severity 6/10 and high confidence (9/10), this introduces real regression risk for observability/debugging because exception type/message/stack data can become inaccurate or lost.
  • Given the issue is scoped to telemetry shaping (not an obvious runtime crash path), this looks manageable but still carries enough user-impacting diagnostic risk to warrant caution.
  • Pay close attention to src/ipc/utils/telemetry.ts - ensure $exception_* fields cannot be overridden by context spread order.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/ipc/utils/telemetry.ts">

<violation number="1" location="src/ipc/utils/telemetry.ts:43">
P2: The `...context` spread is placed after the `$exception_*` properties, which means any context containing keys like `$exception_type`, `$exception_message`, or `$exception_stack_trace_raw` will silently overwrite the actual exception data sent to PostHog. Move `...context` before the exception properties so the real error metadata always takes precedence.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds exception tracking for IPC handlers, which is a valuable improvement for observability. However, the current implementation of sending IPC handler errors to PostHog as telemetry events poses a privacy risk, as it sends raw error messages and stack traces that may contain sensitive information such as local usernames (PII) and user-supplied data from IPC arguments. Additionally, there is a suggestion to improve the consistency of error propagation in one of the handlers to make the overall error handling more robust.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 4, 2026

🎭 Playwright Test Results

❌ Some tests failed

OS Passed Failed Flaky Skipped
🍎 macOS 236 3 7 6

Summary: 236 passed, 3 failed, 7 flaky, 6 skipped

Failed Tests

🍎 macOS

  • debugging_logs.spec.ts > console logs should appear in the console
    • Error: expect(locator).toBeVisible() failed
  • partial_response.spec.ts > partial message is resumed
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • select_component.spec.ts > select component next.js
    • Error: expect(locator).toBeVisible() failed

📋 Re-run Failing Tests (macOS)

Copy and paste to re-run all failing spec files locally:

npm run e2e \
  e2e-tests/debugging_logs.spec.ts \
  e2e-tests/partial_response.spec.ts \
  e2e-tests/select_component.spec.ts

⚠️ Flaky Tests

🍎 macOS

  • annotator.spec.ts > annotator - capture and submit screenshot (passed after 1 retry)
  • debugging_logs.spec.ts > network requests and responses should appear in the console (passed after 1 retry)
  • local_agent_code_search.spec.ts > local-agent - code search (passed after 1 retry)
  • logs_server.spec.ts > system messages UI shows server logs with correct type (passed after 1 retry)
  • setup_flow.spec.ts > Setup Flow > setup banner shows correct state when node.js is installed (passed after 1 retry)
  • setup.spec.ts > setup ai provider (passed after 1 retry)
  • undo.spec.ts > undo (passed after 1 retry)

📊 View full report

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 4, 2026

✅ Claude Code completed successfully

Summary

  • Fixed 2 code review comments with code changes (spread order in sendTelemetryException, error propagation in safe_handle.ts)
  • Resolved 2 review comments with explanations (validation capture scope, PII concern)
  • All 5 review threads resolved
  • E2E test failures are pre-existing flaky tests unrelated to this PR (also failing on main)
Review Comments Details

Addressed with code changes (3 threads):

  • devin-ai-integration + cubic-dev-ai: Fixed ...context spread order in sendTelemetryException so context properties cannot overwrite $exception_* PostHog properties
  • gemini-code-assist: Fixed error propagation in safe_handle.ts to re-throw original error directly instead of wrapping it in new Error()

Resolved with explanations (2 threads):

  • chatgpt-codex-connector: Validation failures from safeParse are intentional contract-mismatch errors, not uncaught handler errors — out of scope for this PR
  • gemini-code-assist: PII concern about stack traces — Electron packaged apps use installation paths, not user home dirs; PostHog exception capture is standard practice

CI Checks:

  • Build: ✅ Passing
  • E2E tests: ❌ Flaky failures in unrelated tests (partial_response, select_component, setup_flow, undo, logs_server, setup, local_agent_code_search) — same pattern of flaky failures seen on main branch

Workflow run

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 4, 2026

🤖 Claude Code Review Summary

PR Confidence: 4/5

All review comments addressed; code changes are minimal and correct. E2E flaky tests reduce confidence slightly but are unrelated to this PR.

Unresolved Threads

No unresolved threads

Resolved Threads

Issue Rationale Link
Context spread overwrites exception properties Valid bug — moved ...context before $exception_* props so exception data always takes precedence View
Error wrapping in safe_handle.ts loses original stack Valid — changed to re-throw original error for consistency with base.ts View
Capture validation failures in telemetry Out of scope — validation errors are intentional contract mismatches, not uncaught handler errors View
PII in stack traces sent to PostHog Standard practice — packaged Electron apps use installation paths, not user home dirs View
Product Principle Suggestions

No suggestions — principles were not needed for these purely technical review comments.


🤖 Generated by Claude Code

@github-actions github-actions bot added cc:done needs-human:review-issue ai agent flagged an issue that requires human review and removed cc:pending labels Mar 4, 2026
@wwwillchen wwwillchen merged commit bfeaebf into dyad-sh:main Mar 5, 2026
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cc:done cc:request needs-human:review-issue ai agent flagged an issue that requires human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant