Log ssh send failure#315787
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds logging to the SSH relay protocol transport so that failures to send messages over the SSH relay are surfaced via the VS Code logging infrastructure, and wires up dependency injection to provide an ILogService to the transport.
Changes:
- Inject
ILogServiceintoSSHRelayTransportand log relay close + relay send failures. - Switch SSH relay transport construction to
IInstantiationService.createInstance(...)to satisfy the new service dependency. - Update SSH relay transport unit tests to pass a
NullLogServicewhen constructing the transport directly.
Show a summary per file
| File | Description |
|---|---|
| src/vs/platform/agentHost/test/electron-browser/sshRelayTransport.test.ts | Updates tests to pass a NullLogService due to the new ILogService dependency. |
| src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts | Uses DI (createInstance) to construct SSHRelayTransport with injected services. |
| src/vs/platform/agentHost/electron-browser/sshRelayTransport.ts | Adds ILogService injection and logs relay close + relaySend failures. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/platform/agentHost/electron-browser/sshRelayTransport.ts:67
relaySendfailures are commented as a likely/expected consequence of a closed connection in the similar TunnelRelayTransport implementation, but here they are logged at error level. This risks log spam during normal teardown or transient disconnects. Consider either (a) logging attrace/warnwiththis._connectionIdincluded, (b) suppressing known/expected errors (e.g. disposed/canceled), or (c) rate-limiting to once per connection.
send(message: ProtocolMessage | AhpServerNotification | JsonRpcNotification | JsonRpcResponse | JsonRpcRequest): void {
const text = JSON.stringify(message);
this._ahpLogger?.log(message, 'c2s', getAhpLogByteLength(text));
this._sshService.relaySend(this._connectionId, text).catch((err) => {
this._logService.error('[SSHRelayTransport] relaySend failed', err);
});
- Files reviewed: 3/3 changed files
- Comments generated: 1
auto-merge was automatically disabled
May 11, 2026 17:43
Pull request was converted to draft
eremognosis
reviewed
May 11, 2026
eremognosis
left a comment
There was a problem hiding this comment.
Looks Good
It logs actually well
No issues found
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Screenshot ChangesBase: Changed (6) |
justschen
approved these changes
May 12, 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.
No description provided.