From 105fbb48e7ec13796a1e090a81d179c13b0ceb62 Mon Sep 17 00:00:00 2001 From: Rolf Heij Date: Thu, 16 Jul 2026 21:56:14 +0200 Subject: [PATCH 1/2] chore(network): log why request-handler unregistration fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At app quit, UnsubscriberAsyncList logs a bare "Unsubscriber at index N failed!" with no context when a registerCommand unregistration resolves false instead of throwing. Add logger.warn calls at each silent false-returning branch (jsonRpc unset in the network.service closure; method not locally registered or remote UNREGISTER_METHOD round-trip failed in rpc-client's unregisterMethod) so the next occurrence self-describes. No behavior change — return values, ordering, and error propagation are identical. Co-Authored-By: Claude Fable 5 --- src/client/services/rpc-client.ts | 11 +++++++++-- src/shared/services/network.service.ts | 9 +++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/client/services/rpc-client.ts b/src/client/services/rpc-client.ts index d7e668e4bed..d2d2779936d 100644 --- a/src/client/services/rpc-client.ts +++ b/src/client/services/rpc-client.ts @@ -176,12 +176,19 @@ export class RpcClient implements IRpcMethodRegistrar { } async unregisterMethod(methodName: string): Promise { - if (!this.jsonRpcServer.hasMethod(methodName)) return false; + if (!this.jsonRpcServer.hasMethod(methodName)) { + logger.warn(`Cannot unregister RPC method ${methodName}: not locally registered`); + return false; + } const mutex = this.registrationMutexMap.get(methodName); return mutex.runExclusive(async () => { - if (!this.jsonRpcServer.hasMethod(methodName)) return false; + if (!this.jsonRpcServer.hasMethod(methodName)) { + logger.warn(`Cannot unregister RPC method ${methodName}: not locally registered`); + return false; + } const successful = await this.jsonRpcClient.request(UNREGISTER_METHOD, [methodName]); if (successful) this.jsonRpcServer.removeMethod(methodName); + else logger.warn(`Remote failed to unregister RPC method ${methodName}`); return successful; }); } diff --git a/src/shared/services/network.service.ts b/src/shared/services/network.service.ts index 7b4cd188195..a3d00200031 100644 --- a/src/shared/services/network.service.ts +++ b/src/shared/services/network.service.ts @@ -317,9 +317,14 @@ export async function registerRequestHandler( if (requestHandlerOptions?.timeoutMilliseconds !== undefined) setTimeoutMsForRequestType(requestType, requestHandlerOptions.timeoutMilliseconds); return async () => { - if (!jsonRpc) return false; + if (!jsonRpc) { + logger.warn(`Could not unregister request handler for "${requestType}": jsonRpc is not set`); + return false; + } removeTimeoutMsForRequestType(requestType); - return jsonRpc.unregisterMethod(requestType); + const unregistered = await jsonRpc.unregisterMethod(requestType); + if (!unregistered) logger.warn(`Failed to unregister request handler for "${requestType}"`); + return unregistered; }; } From 0e394a0897ece7dc4f9024625b4229526003399a Mon Sep 17 00:00:00 2001 From: timothy-mccormack Date: Wed, 22 Jul 2026 02:06:04 +0800 Subject: [PATCH 2/2] fix(network): debug-log the expected !jsonRpc unregister skip instead of warn Addresses lyonsil's #2573 review: the !jsonRpc branch is the normal graceful-shutdown path (shutdown() clears jsonRpc before disposing emitters), so warning there fires on every quit. Drop to debug; keep warn for the genuine unregistered === false case. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/shared/services/network.service.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/shared/services/network.service.ts b/src/shared/services/network.service.ts index a3d00200031..6c8a916ae86 100644 --- a/src/shared/services/network.service.ts +++ b/src/shared/services/network.service.ts @@ -318,7 +318,13 @@ export async function registerRequestHandler( setTimeoutMsForRequestType(requestType, requestHandlerOptions.timeoutMilliseconds); return async () => { if (!jsonRpc) { - logger.warn(`Could not unregister request handler for "${requestType}": jsonRpc is not set`); + // Expected on graceful shutdown: shutdown() clears jsonRpc before disposing emitters so their + // disposers skip this now-pointless unregister, so this fires on every normal quit — debug, + // not warn, to avoid spurious teardown noise (mirrors disposeNetworkEventEmitter's quiet + // !jsonRpc return). The genuine failure to warn about is the unregistered === false case below. + logger.debug( + `Skipping unregister of request handler for "${requestType}": jsonRpc is not set`, + ); return false; } removeTimeoutMsForRequestType(requestType);