Repository navigation
fix: log named pipe disconnects and timeouts in FrontendRunner requests - #285
Merged
baxeaz merged 1 commit intoNov 3, 2026
Conversation
On Windows, FrontendRunner._send_request logged "Failed to send {path}
request" only for pywintypes.error. NamedPipeHelper raises its own
PipeDisconnectedException and NamedPipeTimeoutError, which do not derive
from pywintypes.error, so a broken pipe or read timeout during a heartbeat
propagated without saying which request failed.
Catch those exceptions as well, and include the elapsed time so the log
shows whether the backend closed the connection at once or after its own
request read timeout.
Signed-off-by: Brian Axelson <86568017+baxeaz@users.noreply.github.com>
|
Claude review · advisory Reviewed Open: 0 blocking · 0 should-fix · 0 nit. ✅ Nothing blocking. Fix or reply to each thread; the next revision's review re-checks open threads and resolves those it agrees are handled. Resolving a thread yourself also closes it. Later revisions review only what changed. |
leongdl
approved these changes
Nov 2, 2026
mwiebe
approved these changes
Nov 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was the problem/requirement? (What/Why)
On Windows,
FrontendRunner._send_requestlogsFailed to send {path} request: ...only when the named pipe call raisespywintypes.error.NamedPipeHelperraises its ownPipeDisconnectedExceptionandNamedPipeTimeoutErrorfor a broken pipe or a read/connect timeout, and neither derives frompywintypes.error. Those errors propagate without any log line saying which request failed or how long it took.We hit this with an adaptor whose foreground command failed during a long startup with only:
That error means the backend closed the connection without replying. The backend does this when its 5s request read in
ResourceRequestHandler.instance_threadtimes out, when a handle is closed without a response, or when the backend process exits. Telling these apart needs the request path and how long the frontend waited.What was the solution? (How)
_send_requestnow also catchesPipeDisconnectedExceptionandNamedPipeTimeoutError, logs them the same way aspywintypes.error, and re-raises. Behavior is otherwise unchanged.Failed to send /heartbeat request after 5.02s: An error occurred: The pipe has been ended. (Error code: 109) in function ReadFile. A disconnect after about 5s points at the backend's request read timeout; a disconnect after a few milliseconds points at a closed handle or an exited backend.What is the impact of this change?
One extra error log line on the frontend when a named pipe request fails on Windows. The same exceptions are raised as before. POSIX is unaffected.
How was this change tested?
See DEVELOPMENT.md for information on running tests.
test_raises_when_request_failsinTestSendRequestInWindowsis now parametrized overpywintypes.error,PipeDisconnectedException, andNamedPipeReadTimeoutError, andchecks the logged path, elapsed time, and error text.
test_frontend_runner.pypasses (26 passed, 5 skipped). With the source change reverted, all three parametrized cases fail, so the test covers the change. The rest of the unit suite passed except two tests that fail the same way without this change in that environment (the session ran as SYSTEM, whereLookupAccountNamereturns 1332, plus an argparse difference intest_raises_jsonschema_validation_err).hatch run testfortest_frontend_runner.pypasses (Windows cases skipped);hatch run lintpasses.NamedPipeReadTimeoutErrorinstead.Was this change documented?
Is this a breaking change?
No.
Does this change impact security?
No. The log line contains the request path and the exception message, which were already logged for
pywintypes.error. No request bodies or parameters are logged.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.