Skip to content

fix: Attach error handler to RPC stream#3898

Merged
FrederikBolding merged 5 commits intomainfrom
fb/uncaught-error-on-snap-crash
Mar 11, 2026
Merged

fix: Attach error handler to RPC stream#3898
FrederikBolding merged 5 commits intomainfrom
fb/uncaught-error-on-snap-crash

Conversation

@FrederikBolding
Copy link
Member

@FrederikBolding FrederikBolding commented Mar 11, 2026

When the execution environment fails on mobile (we still don't know the cause of this) we were seeing uncaught "Premature close" errors, which ends up crashing the app.

The issue is that all streams are destroyed when the Snap terminates (as it does if it cannot initialize). Most streams already have error handling setup via pipeline and if the Snap in question has already run this applies to the RPC stream too. The problem is that if the Snap never starts, the RPC stream also never has any error handlers attached, so instead it bubbles up the error.

This PR fixes that by attaching an error handler to the RPC stream after it has been created.

https://consensyssoftware.atlassian.net/browse/WPC-516


Note

Medium Risk
Touches snap execution stream lifecycle in AbstractExecutionService, which is central to snap RPC plumbing; while the change is small, it could affect error propagation/logging during initialization and teardown.

Overview
Prevents uncaught RPC stream errors during snap startup/termination by attaching an error listener to the JSON-RPC mux stream as soon as it’s created, filtering out expected Premature close noise while logging other failures.

Adds a regression test that simulates a failed/unreachable execution environment and asserts the request fails cleanly without throwing Premature close, and relaxes test utilities to accept any AbstractExecutionService instance for injection.

Written by Cursor Bugbot for commit c8458fa. This will update automatically on new commits. Configure here.

@FrederikBolding FrederikBolding changed the title wip: Uncaught error on execution environment failure fix: Attach error handler to RPC stream Mar 11, 2026
@FrederikBolding FrederikBolding marked this pull request as ready for review March 11, 2026 13:49
@FrederikBolding FrederikBolding requested a review from a team as a code owner March 11, 2026 13:49
Comment on lines -694 to +695
service?: ReturnType<typeof getNodeEES>,
service?: AbstractExecutionService<unknown>,
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't need to be a NodeThreadExecutionService, it should allow any type

Copy link

@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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.55%. Comparing base (5a88d61) to head (c8458fa).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ntrollers/src/services/AbstractExecutionService.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3898      +/-   ##
==========================================
- Coverage   98.56%   98.55%   -0.01%     
==========================================
  Files         425      425              
  Lines       12355    12358       +3     
  Branches     1934     1935       +1     
==========================================
+ Hits        12178    12180       +2     
- Misses        177      178       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@FrederikBolding FrederikBolding requested a review from Mrtenz March 11, 2026 14:02
@FrederikBolding FrederikBolding added this pull request to the merge queue Mar 11, 2026
Merged via the queue into main with commit 08299db Mar 11, 2026
128 of 130 checks passed
@FrederikBolding FrederikBolding deleted the fb/uncaught-error-on-snap-crash branch March 11, 2026 14:09
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.

2 participants