feat: global --debug HTTP request/response logging#29
Merged
Conversation
Add a `--debug transport` mode that dumps raw HTTP requests and responses to stderr for diagnostics, with no per-command wiring. The `--debug` flag was already parsed into `Middleware.debug` but never consumed. This wires it up: - `flags::debug_component_enabled` parses the comma-separated component pattern (`*`, `transport`, `*,-auth`) — the first consumer of the syntax. - A process-global default `TransportLogger` (mirroring `set_default_user_agent`) is published when `--debug` selects `transport`; every `HttpClient` built afterward inherits it as its default logger, so handlers get a curl-style trace automatically. An explicit `HttpClientBuilder::logger` still overrides. - `TransportLogEvent` gains optional `headers`/`body`; the transport send paths capture request headers+body, and response buffering/logging is centralized so JSON/decode responses are dumped in full. Raw byte-download and streaming responses log size only. Raw/multipart/if-match paths now log responses too. - `StderrTransportLogger` renders the trace and redacts sensitive headers (authorization, proxy-authorization, cookie, set-cookie, x-api-key) by default. `CliConfig::with_redacted_debug_headers` adds CLI-specific secret-bearing headers (additive, case-insensitive). - `transport::debug_log_reqwest_request`/`debug_log_reqwest_response` feed the same global sink from bare `reqwest` / progenitor clients that cannot use `HttpClient`. docs/concepts.md documents the feature and the gaps that would let more bare-reqwest call sites migrate onto HttpClient. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class support for --debug transport by introducing a process-global transport logger, capturing request/response headers and (when safe) bodies, and providing a stderr renderer with header redaction—so HTTP diagnostics work automatically across commands without per-command wiring.
Changes:
- Implemented
flags::debug_component_enabledto parse the documented comma-separated--debugcomponent patterns (*,transport,*,-auth, etc.). - Added a process-wide default transport logger and expanded transport events to optionally include headers/body; introduced
StderrTransportLoggerto render redacted curl-style traces to stderr. - Updated docs and tests to reflect unified request/response events and default-logger inheritance behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/foundation.rs | Updates transport logging expectations and adds an integration test for process-global default logger inheritance. |
| src/transport/mod.rs | Wires in the new debug logger module and re-exports the logger + reqwest-direct debug helpers. |
| src/transport/debug_logger.rs | Adds StderrTransportLogger to render redacted request/response traces to stderr. |
| src/transport/client.rs | Introduces process-global default transport logger, adds headers/body to TransportLogEvent, and centralizes response buffering/logging. |
| src/lib.rs | Re-exports debug_component_enabled from flags at the crate root. |
| src/flags.rs | Implements debug_component_enabled and adds unit coverage for pattern parsing. |
| src/cli.rs | Adds CLI config for extra redacted headers and installs/resets the process-global logger based on --debug. |
| docs/concepts.md | Documents --debug component patterns and the new HTTP debug logging behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- set_default_transport_logger / default_transport_logger now recover a poisoned RwLock (PoisonError::into_inner) instead of silently no-oping, so a panic elsewhere can't leave a stale logger installed. - StderrTransportLogger renders status-only responses (from the reqwest-direct helper) as `< 204` without trailing spaces; added a regression test. - Replaced `drop(write_all(..))` with `.ok()` (plain `let _ =` would trip the crate's `let_underscore_drop` lint). - Serialized the process-global-logger integration test behind a new TRANSPORT_LOGGER_TEST_LOCK so its install/assert/reset window is isolated. - Fixed a missing space in docs/concepts.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address Copilot re-review: - Add `TransportLogger::enabled()` (default true; `NoopTransportLogger` returns false). `log_request`/`log_response` skip capture when disabled, and `read_and_log_response` no longer clones response headers on the non-debug path. - `read_and_log_response` now returns `bytes::Bytes` (a cheap clone of the buffer reqwest already owns) instead of `Vec<u8>`, so JSON decode parses without an extra full-body copy. Added `bytes` as a direct dependency. - `retryable_status_error` only clones headers when logging is enabled. - Soften the debug_logger module docs: redaction covers sensitive headers, but URLs and bodies are printed in full and may still contain secrets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
URLs and request/response bodies are printed in full and may contain secrets even with headers redacted; warn rather than calling the output ticket-safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
debug_log_reqwest_request / debug_log_reqwest_response now fetch the default logger once and return early when it is disabled, so reqwest-direct call sites clone no headers and copy no body on the non-debug path — consistent with HttpClient::log_request/log_response. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address Copilot re-review: - ensure_success_response no longer buffers the body on a non-error status when logging is disabled, restoring the pre-logging behavior where *_without_response calls never read success bodies. The body is read only when it's needed — to build an error message or to feed an enabled logger. - read_and_log_response's body-read failure now reports "read response body" instead of "decode response", which was misleading on raw/byte-download paths (get_raw/get_bytes/post_raw). JSON parse errors still say "decode". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Redaction is a safety feature, so a config value with stray whitespace (or an empty entry) must not silently fail to match and leak the header. Both StderrTransportLogger::with_redacted_headers and CliConfig::with_redacted_debug_headers now trim names and drop empties. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An empty/whitespace component name is now never enabled, even by `*`, so the public helper can't return true for an invalid component query. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The last remaining capture site: log_debug built the fields map even when the logger was disabled. Now every transport logging entry point skips allocation on the non-debug path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot re-review: - The `--debug transport` space form does not work before the subcommand: `normalize_optional_global_flags_before_command` treats `--debug` as an optional-value flag at the root and rewrites it to `--debug=*`, leaving the next token positional (so `--debug transport <cmd>` fails). Verified at runtime. Docs now use the forms that work at root — bare `--debug` (all), `--debug=transport`, `--debug='*,-transport'` — and note the space form only attaches a value after the leaf command. - install_debug_transport_logger test now asserts on `TransportLogger::enabled()` instead of the logger's `Debug` string, which is brittle to formatting/field changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jbrooks2-godaddy
approved these changes
Jun 23, 2026
runruh-godaddy
approved these changes
Jun 23, 2026
jbrooks2-godaddy
pushed a commit
that referenced
this pull request
Jun 24, 2026
🤖 I have created a release *beep* *boop* --- ## [0.3.2](cli-engine-v0.3.1...cli-engine-v0.3.2) (2026-06-24) ### Features * global --debug HTTP request/response logging ([#29](#29)) ([7f4dbb2](7f4dbb2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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
Adds a global
--debug transportmode that dumps raw HTTP requests and responses to stderr for diagnostics — automatically, with no per-command wiring.The
--debugflag was already parsed intoMiddleware.debugbut never consumed. This PR makes it real.Usage: bare
--debugenables all debug components; select one with the=form so the value isn't taken as the command —--debug=transport,--debug='*,-transport'. (As an optional-value global flag, the space-separated--debug transportform only attaches a value when it appears after the leaf command.)How
flags::debug_component_enabledparses the comma-separated component pattern (*,transport,*,-auth). Fails closed on an empty component name.TransportLogger(set_default_transport_logger/default_transport_logger), mirroring the existingset_default_user_agentprecedent. When--debugselectstransport, the engine publishes aStderrTransportLogger; everyHttpClientbuilt afterward inherits it as its default logger. An explicitHttpClientBuilder::loggerstill overrides per client. Installed once per invocation, before the handler runs, so all of a command's HTTP requests are logged. Recovers from a poisoned lock rather than silently no-oping.TransportLogEventgains optionalheaders/body. Send paths capture request headers+body; response buffering/logging is centralized so JSON/decode responses are dumped in full. Raw byte-download and streaming responses log size only (no large-payload buffering). Raw/multipart/if-match paths now log responses too (previously request-only).StderrTransportLoggerrenders a curl-style trace and redactsauthorization,proxy-authorization,cookie,set-cookie,x-api-keyby default.CliConfig::with_redacted_debug_headersadds CLI-specific secret-bearing headers (additive, case-insensitive, names trimmed / empties dropped so a mistyped value can't silently leak) — e.g. gdx'sx-litellm-api-key. URLs and bodies are printed in full and may still contain secrets, so the output is documented as sensitive (not "ticket-safe").transport::debug_log_reqwest_request/debug_log_reqwest_responsefeed the same global sink from barereqwestand progenitor-generated clients that cannot sit onHttpClient.TransportLogger::enabled()(defaulttrue;NoopTransportLoggerreturnsfalse) gates every capture site, so a normal run clones no headers and copies no bodies.read_and_log_responsereturnsbytes::Bytes(added as a direct dep) so JSON decode parses without an extra copy;*_without_responsesuccess bodies are not read when logging is off.Consumer impact
HttpClient) gets coverage automatically; add.with_redacted_debug_headers(["x-litellm-api-key"])for its one custom secret header.HttpClientnowhere and its progenitordomains_clientcan't adopt it — the reqwest helpers give it a path without migration.docs/concepts.mdrecords the engine additions a future migration would need.TransportLogEventfields, newCliConfigfield, newTransportLogger::enabled()default method); ships as a 0.x minor (feat) release. Verified neither consumer repo constructsTransportLogEvent/CliConfigvia struct literals.Tests
debug_component_enabledmatrix (incl. empty-component fail-closed);StderrTransportLoggerredaction/body formatting, configured extra-header redaction (trim/empty/case), status-only response rendering;--debug→logger install wiring (asserts onenabled()).tests/foundation.rs): a default-loggerHttpClientinherits the process-global logger (serialized behind a per-binary lock); existing logger sequence tests updated for the unified events and body/header capture.cargo fmt --all --check,cargo clippy --all-targets -- -D warnings,RUSTDOCFLAGS='-D warnings' cargo doc --no-deps,cargo test --all-targets,cargo test --doc, missing-docs = 0 — all green.🤖 Generated with Claude Code