[Audit-Medium] failed-requests ignores normal non-OK responses, so fa…

[pepone]

AI-generated audit finding — this issue was opened from an automated security/correctness audit. It has not been triaged by a human yet; verify the reasoning, reproducibility, and severity before acting on it.

Medium: failed-requests ignores normal non-OK responses, so failure metrics depend on whether an error is returned or thrown — CONFIRMED

Affected code:

• src/IceRpc.Metrics/MetricsInterceptor.cs:27-48 — only thrown exceptions classify; IncomingResponse with a non-OK StatusCode returns through the unmodified success path
• src/IceRpc.Metrics/MetricsMiddleware.cs:27-50 — same pattern for dispatch
• src/IceRpc/Router.cs:193, src/IceRpc/Internal/NotFoundDispatcher.cs:13 — router returns OutgoingResponse(StatusCode.NotFound) rather than throwing
• src/IceRpc.Deadline/DeadlineMiddleware.cs:47-50 — deadline middleware returns OutgoingResponse(StatusCode.DeadlineExceeded)
• src/IceRpc/Internal/IceRpcProtocolConnection.cs:570, src/IceRpc/Internal/IceProtocolConnection.cs:756 — client surfaces non-OK responses as a normal IncomingResponse

Verification:

Confirmed. The implementation is literally try { return await _next(...) } catch (OperationCanceledException) { RequestCancel(); throw; } catch { RequestFailure(); throw; } — no StatusCode inspection anywhere. The library's own dispatchers (Router.NotFoundDispatcher) and middleware (DeadlineMiddleware.DispatchAsync) deliberately return a non-OK response rather than throwing, so a typical production setup that uses the deadline middleware or the router's default "not found" behavior already has a meaningful share of real failures invisible to failed-requests.

Impact:

• failed-requests materially under-reports real failures on both client and server sides.
• Two logically identical failure paths (thrown vs. returned) produce different metrics.
• Dashboards and alerts built on failed-requests are unreliable.

Recommendation:

• Inspect the outcome's StatusCode and, if non-OK, increment failed-requests before returning.
  • Client: after the await _next.InvokeAsync(...), check response.StatusCode != StatusCode.Ok.
  • Server: similarly for OutgoingResponse.StatusCode.
• Consider whether to treat DispatchException-derived responses differently from unexpected failures — this is the same classification question the Logger audit flags for LoggerMiddleware.
• Add regression tests for: client receiving IncomingResponse(StatusCode.NotFound), router returning OutgoingResponse(StatusCode.NotFound), DeadlineMiddleware returning OutgoingResponse(StatusCode.DeadlineExceeded).

Status: Valid, Medium severity.

---

Source report: src-IceRpc.Metrics-audit-2026-04-14.md (finding ``failed-requests ignores normal non-OK responses, so failure metrics depend on whether an error is returned or thrown — **CONFIRMED**)

Severity (auditor-assigned): Medium