Skip to content

Commit e575b0e

Browse files
fix(sidecar+protocols): typed catches with discard on shutdown swallow paths (cs/empty-catch-block)
Six bare-or-typed-but-empty catch blocks across sidecar teardown (SidecarHttpTransport, SidecarBowireProtocol, SidecarJsonRpcTransport x2), gRPC send loop, and the SocketIO discovery probe. Each catches a genuinely expected exit path (cancellation during dispose, dead old transport, closed stdin on a reaped child process, caller-cancel on the channel, abort during the 2s discovery delay) where propagating would either crash Dispose or block recovery. Converted bare 'catch { }' to typed 'catch (Exception ex)' and added a discard '_ = ex' to give CodeQL a real statement plus a comment block explaining why the swallow is intentional. Behaviour identical.
1 parent e9a8bf4 commit e575b0e

5 files changed

Lines changed: 43 additions & 6 deletions

File tree

src/Kuestenlogik.Bowire.Protocol.Grpc/GrpcBowireChannel.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,11 @@ private async Task RunDuplexAsync(Method<byte[], byte[]> rawMethod)
272272
await call.RequestStream.WriteAsync(msg);
273273
await call.RequestStream.CompleteAsync();
274274
}
275-
catch (OperationCanceledException) { }
275+
catch (OperationCanceledException ex)
276+
{
277+
// Caller closed the channel -- expected exit path.
278+
_ = ex;
279+
}
276280
}, token);
277281

278282
await foreach (var responseBytes in call.ResponseStream.ReadAllAsync(token))

src/Kuestenlogik.Bowire.Protocol.SocketIo/BowireSocketIoProtocol.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,13 @@ public async Task<List<BowireServiceInfo>> DiscoverAsync(
6060
client.OnAny((name, _) => { detectedEvents.Add(name); return Task.CompletedTask; });
6161

6262
await client.ConnectAsync();
63-
try { await Task.Delay(2000, ct); } catch (OperationCanceledException) { }
63+
try { await Task.Delay(2000, ct); }
64+
catch (OperationCanceledException ex)
65+
{
66+
// Discovery probe cut short -- we still want the
67+
// disconnect call below, so swallow rather than throw.
68+
_ = ex;
69+
}
6470
await client.DisconnectAsync();
6571

6672
if (!connected) return [];

src/Kuestenlogik.Bowire/Plugins/Sidecar/SidecarBowireProtocol.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,14 @@ internal async Task<ISidecarTransport> EnsureStartedAsync(CancellationToken ct)
249249
// If a previous run died, dispose it before respawning.
250250
if (_transport is not null)
251251
{
252-
try { await _transport.DisposeAsync().ConfigureAwait(false); } catch { }
252+
try { await _transport.DisposeAsync().ConfigureAwait(false); }
253+
catch (Exception ex)
254+
{
255+
// Old transport already dead -- disposal is best-
256+
// effort. We're about to spawn a fresh one anyway,
257+
// so a clean-up failure here doesn't block recovery.
258+
_ = ex;
259+
}
253260
_transport = null;
254261
_initResult = null;
255262
}

src/Kuestenlogik.Bowire/Plugins/Sidecar/SidecarHttpTransport.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,14 @@ public async ValueTask DisposeAsync()
185185
await _lifetime.CancelAsync().ConfigureAwait(false);
186186
if (_sseLoop is not null)
187187
{
188-
try { await _sseLoop.ConfigureAwait(false); } catch { }
188+
try { await _sseLoop.ConfigureAwait(false); }
189+
catch (Exception ex)
190+
{
191+
// Best-effort wait for the SSE loop to observe our
192+
// cancellation; the loop is already torn down, so any
193+
// exception (cancel, network reset, &c) is irrelevant.
194+
_ = ex;
195+
}
189196
}
190197
_lifetime.Dispose();
191198
_http.Dispose();

src/Kuestenlogik.Bowire/Plugins/Sidecar/SidecarJsonRpcTransport.cs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,13 @@ public async ValueTask DisposeAsync()
251251
catch { /* swallow — we're killing the process anyway */ }
252252

253253
try { _process.StandardInput.Close(); }
254-
catch { }
254+
catch (Exception ex)
255+
{
256+
// Process may already be gone (stream closed, disposed,
257+
// or kernel reaped it) -- stdin-close is just a polite
258+
// EOF signal, not a correctness requirement.
259+
_ = ex;
260+
}
255261

256262
try
257263
{
@@ -270,7 +276,14 @@ public async ValueTask DisposeAsync()
270276

271277
if (_readLoop is not null)
272278
{
273-
try { await _readLoop.ConfigureAwait(false); } catch { }
279+
try { await _readLoop.ConfigureAwait(false); }
280+
catch (Exception ex)
281+
{
282+
// Read loop normally exits cleanly when the process
283+
// closes stdout; if it didn't (cancel race, pipe error),
284+
// we're already in disposal so propagating doesn't help.
285+
_ = ex;
286+
}
274287
}
275288
_process.Dispose();
276289
}

0 commit comments

Comments
 (0)