Add request/response debug logging#23
Conversation
Introduce ShadeClient with debug mode and global config so developers can inspect masked HTTP traffic during integration troubleshooting.
Resolve conflicts in __init__.py (combine exports), test_gateway.py (keep patch import, drop unused pytest), and regenerate poetry.lock.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesRequest/response debug logging
Roadmap and backlog docs
Sequence Diagram(s)sequenceDiagram
participant ShadeClient
participant shade._debug
participant httpx.Client
participant logger
ShadeClient->>shade._debug: log_request(method, url, headers, body)
ShadeClient->>httpx.Client: request(method, url, headers, json, content)
httpx.Client-->>ShadeClient: httpx.Response
ShadeClient->>shade._debug: log_response(status_code, headers, body)
shade._debug->>logger: logger.debug(...)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shade/client.py`:
- Line 50: The URL composition in Client should normalize the path before
joining it with base_url, because using a raw path can produce invalid URLs when
the path is missing a leading slash. Update the logic around the url assignment
in Client to ensure path is normalized consistently before concatenation, and
keep the fix localized to the URL-building code in this method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: add01f4d-79ca-40d3-9a34-a54efac4e31e
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
pyproject.tomlsrc/shade/__init__.pysrc/shade/_debug.pysrc/shade/client.pysrc/shade/config.pytests/test_debug_logging.pytests/test_gateway.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/shade/gateway.py (1)
45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new public constructor options.
debugandhttp_clientare public SDK inputs, but theGatewaydocstring omits their behavior and ownership semantics.Suggested docstring addition
timeout : float Per-request socket timeout in seconds. + debug : bool + Enable DEBUG-level request/response logging via the ``shade`` logger. + http_client : httpx.Client, optional + Custom synchronous HTTP client. When supplied, ownership remains with + the caller and Gateway must not close it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shade/gateway.py` around lines 45 - 46, The Gateway constructor now accepts public SDK inputs that are not documented, so update the Gateway docstring to describe the behavior of debug and http_client. Note that debug should explain its effect on logging/diagnostics, and http_client should clarify that callers may pass a custom httpx.Client and who owns its lifecycle. Use the Gateway class and its __init__ signature to keep the docs aligned with the public API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ISSUES.md`:
- Around line 1267-1269: The SwapPayment contract is out of sync with the
endpoint payload because it expects payment_address but the model spec does not
define that field. Update the SwapPayment model definition referenced by the
checklist so it includes payment_address (and still matches status ==
"pending"), or remove the assertion here if the field is not part of the model
contract. Keep the endpoint and model schema aligned in the SwapPayment-related
definitions.
- Around line 295-299: Broaden the debug-logging acceptance criteria in the spec
so it covers the full logging contract, not just method, URL, headers, status,
and body. Update the checklist items around the debug-logging requirements to
explicitly include request bodies, response headers, and the masking/truncation
rules for sensitive or large values, while keeping the existing
`debug=True`/`debug=False` and `logging.DEBUG` expectations. Ensure the updated
wording aligns with the logging feature’s intended behavior so incomplete
implementations cannot pass.
- Around line 617-629: The webhook event spec is inconsistent about the
`WebhookEvent.data` contract, so update the documentation to choose one behavior
and make it uniform throughout. Align the description, proposed steps, and
acceptance criteria around the intended model in `src/shade/models/webhook.py`
and `WebhookEvent.from_dict(...)`, and make sure the text no longer says `data`
is both a typed model and a raw dict. If the typed-model contract is the
intended one, adjust the acceptance criteria and remove the raw-dict wording so
the spec matches the webhook flow and `Webhook.construct_event()` behavior.
In `@src/shade/gateway.py`:
- Around line 71-76: The Gateway implementation is split between
`Gateway.request()` using `ClientShadeClient` and the existing
`process_payment`/legacy paths using `_http` and `_async_http`, so debug
logging, injected `http_client`, retries, and timeout behavior are inconsistent.
Update `Gateway` and the related request methods so all traffic goes through one
HTTP stack, or extend the existing sync/async clients to accept `debug` and
injected client support while preserving `max_retries` and `timeout` behavior.
Make sure the fix is applied consistently across `Gateway.request`,
`Gateway.process_payment`, and the client initialization in `Gateway.__init__`.
---
Nitpick comments:
In `@src/shade/gateway.py`:
- Around line 45-46: The Gateway constructor now accepts public SDK inputs that
are not documented, so update the Gateway docstring to describe the behavior of
debug and http_client. Note that debug should explain its effect on
logging/diagnostics, and http_client should clarify that callers may pass a
custom httpx.Client and who owns its lifecycle. Use the Gateway class and its
__init__ signature to keep the docs aligned with the public API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c4bfe75-adcd-406f-a07e-78c6e20a0aaa
📒 Files selected for processing (4)
FEATURES.mdISSUES.mdsrc/shade/config.pysrc/shade/gateway.py
| - [ ] With `debug=True`, each request logs method, URL, and masked headers. | ||
| - [ ] Each response logs status code and truncated body. | ||
| - [ ] Authorization header value is always masked in logs. | ||
| - [ ] With `debug=False` (default), no request/response content is logged. | ||
| - [ ] Logs use `logging.DEBUG` level so they don't appear unless the app enables them. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Broaden the debug-logging acceptance criteria.
This only checks request method/URL/headers and response status/body. The feature objective also requires request bodies, response headers, and explicit masking/truncation rules, so this spec would let an incomplete implementation pass.
Suggested spec update
- [ ] With `debug=True`, each request logs method, URL, and masked headers.
- [ ] Each response logs status code and truncated body.
+ [ ] With `debug=True`, each request logs method, URL, headers, and body.
+ [ ] Each response logs status code, headers, and body.
+ [ ] Authorization is masked and response bodies over 2000 chars are truncated with `[truncated]`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [ ] With `debug=True`, each request logs method, URL, and masked headers. | |
| - [ ] Each response logs status code and truncated body. | |
| - [ ] Authorization header value is always masked in logs. | |
| - [ ] With `debug=False` (default), no request/response content is logged. | |
| - [ ] Logs use `logging.DEBUG` level so they don't appear unless the app enables them. | |
| - [ ] With `debug=True`, each request logs method, URL, headers, and body. | |
| - [ ] Each response logs status code, headers, and body. | |
| - [ ] Authorization is masked and response bodies over 2000 chars are truncated with `[truncated]`. | |
| - [ ] With `debug=False` (default), no request/response content is logged. | |
| - [ ] Logs use `logging.DEBUG` level so they don't appear unless the app enables them. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ISSUES.md` around lines 295 - 299, Broaden the debug-logging acceptance
criteria in the spec so it covers the full logging contract, not just method,
URL, headers, status, and body. Update the checklist items around the
debug-logging requirements to explicitly include request bodies, response
headers, and the masking/truncation rules for sensitive or large values, while
keeping the existing `debug=True`/`debug=False` and `logging.DEBUG`
expectations. Ensure the updated wording aligns with the logging feature’s
intended behavior so incomplete implementations cannot pass.
| Represents a parsed, verified webhook event delivered by the Shade platform. The `data` field should be typed as the corresponding resource model rather than a raw dict. | ||
|
|
||
| ## **Proposed Steps:** | ||
| - Create `src/shade/models/webhook.py`. | ||
| - Fields: `id`, `type: str`, `data: Any`, `created_at: datetime`, `livemode: bool`. | ||
| - Add a `WebhookEventType` constants class. | ||
| - The `Webhook.construct_event()` method (issue #66) populates `data` with the typed model. | ||
|
|
||
| ## **Acceptance Criteria:** | ||
| - [ ] `WebhookEvent.from_dict(raw_payload)` populates all fields. | ||
| - [ ] `event.livemode` correctly reflects the event origin. | ||
| - [ ] `event.data` is the raw dict at model level; typed model coercion happens in the resource layer. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve the WebhookEvent.data type contract.
This section contradicts itself: the description says data should be a typed model, but the acceptance criteria say it stays a raw dict and gets typed later. Pick one contract; the rest of the webhook flow already points to typed WebhookEvent.data.
Suggested spec fix
- [ ] `event.data` is the raw dict at model level; typed model coercion happens in the resource layer.
+ [ ] `event.data` is deserialized into the matching typed model based on `event.type`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Represents a parsed, verified webhook event delivered by the Shade platform. The `data` field should be typed as the corresponding resource model rather than a raw dict. | |
| ## **Proposed Steps:** | |
| - Create `src/shade/models/webhook.py`. | |
| - Fields: `id`, `type: str`, `data: Any`, `created_at: datetime`, `livemode: bool`. | |
| - Add a `WebhookEventType` constants class. | |
| - The `Webhook.construct_event()` method (issue #66) populates `data` with the typed model. | |
| ## **Acceptance Criteria:** | |
| - [ ] `WebhookEvent.from_dict(raw_payload)` populates all fields. | |
| - [ ] `event.livemode` correctly reflects the event origin. | |
| - [ ] `event.data` is the raw dict at model level; typed model coercion happens in the resource layer. | |
| Represents a parsed, verified webhook event delivered by the Shade platform. The `data` field should be typed as the corresponding resource model rather than a raw dict. | |
| ## **Proposed Steps:** | |
| - Create `src/shade/models/webhook.py`. | |
| - Fields: `id`, `type: str`, `data: Any`, `created_at: datetime`, `livemode: bool`. | |
| - Add a `WebhookEventType` constants class. | |
| - The `Webhook.construct_event()` method (issue `#66`) populates `data` with the typed model. | |
| ## **Acceptance Criteria:** | |
| - [ ] `WebhookEvent.from_dict(raw_payload)` populates all fields. | |
| - [ ] `event.livemode` correctly reflects the event origin. | |
| - [ ] `event.data` is deserialized into the matching typed model based on `event.type`. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 619-619: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 625-625: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ISSUES.md` around lines 617 - 629, The webhook event spec is inconsistent
about the `WebhookEvent.data` contract, so update the documentation to choose
one behavior and make it uniform throughout. Align the description, proposed
steps, and acceptance criteria around the intended model in
`src/shade/models/webhook.py` and `WebhookEvent.from_dict(...)`, and make sure
the text no longer says `data` is both a typed model and a raw dict. If the
typed-model contract is the intended one, adjust the acceptance criteria and
remove the raw-dict wording so the spec matches the webhook flow and
`Webhook.construct_event()` behavior.
| - [ ] Returns a `SwapPayment` model with a `payment_address` and `status == "pending"`. | ||
| - [ ] `slippage_tolerance` outside `(0, 1)` raises `InvalidRequestError` before the call. | ||
| - [ ] Both `pay_in_token` and `settle_out_token` are required; missing either raises `InvalidRequestError`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align SwapPayment with the returned payload.
#60 requires a payment_address, but the SwapPayment model in #28 never defines that field. Either add it to the model spec or remove it here; otherwise the endpoint and model contracts can't both be satisfied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ISSUES.md` around lines 1267 - 1269, The SwapPayment contract is out of sync
with the endpoint payload because it expects payment_address but the model spec
does not define that field. Update the SwapPayment model definition referenced
by the checklist so it includes payment_address (and still matches status ==
"pending"), or remove the assertion here if the field is not part of the model
contract. Keep the endpoint and model schema aligned in the SwapPayment-related
definitions.
| self._client = ClientShadeClient( | ||
| api_key=api_key, | ||
| base_url=self._base_url, | ||
| debug=debug, | ||
| http_client=http_client, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Avoid splitting Gateway traffic across two HTTP stacks.
debug and http_client only affect Gateway.request(). Existing Gateway APIs still use _http/_async_http, so Gateway(..., debug=True).process_payment(...) will not emit the required logs and injected clients will not see those calls. Conversely, Gateway.request() bypasses the documented max_retries/timeout settings carried by the legacy clients. Unify the request path or add debug/client support to the existing sync/async clients while preserving retry/timeout behavior.
Also applies to: 91-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shade/gateway.py` around lines 71 - 76, The Gateway implementation is
split between `Gateway.request()` using `ClientShadeClient` and the existing
`process_payment`/legacy paths using `_http` and `_async_http`, so debug
logging, injected `http_client`, retries, and timeout behavior are inconsistent.
Update `Gateway` and the related request methods so all traffic goes through one
HTTP stack, or extend the existing sync/async clients to accept `debug` and
injected client support while preserving `max_retries` and `timeout` behavior.
Make sure the fix is applied consistently across `Gateway.request`,
`Gateway.process_payment`, and the client initialization in `Gateway.__init__`.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shade/config.py`:
- Around line 16-34: The Config.environment assignment currently bypasses
parsing, so string values like "production" can be stored raw and later break
Gateway when it expects an Environment with base_url. Update the Config class so
environment is a parsed property like api_base, and route all assignments
through parse_environment while preserving the existing Environment enum
handling and validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cc230946-6837-47d6-9a72-c85903372809
📒 Files selected for processing (3)
src/shade/__init__.pysrc/shade/config.pysrc/shade/gateway.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/shade/init.py
- src/shade/gateway.py
| self.environment: Environment = Environment.SANDBOX | ||
|
|
||
| @property | ||
| def api_base(self) -> Optional[str]: | ||
| return self._api_base | ||
|
|
||
| @api_base.setter | ||
| def api_base(self, value: Optional[str]) -> None: | ||
| self._api_base = value | ||
|
|
||
| def parse_environment(self, value: str | Environment) -> Environment: | ||
| if isinstance(value, Environment): | ||
| return value | ||
| if isinstance(value, str): | ||
| try: | ||
| return Environment(value.lower()) | ||
| except ValueError: | ||
| pass | ||
| raise ValueError("Invalid environment. Valid options are: 'sandbox', 'production'") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make environment a parsed property on Config.
Because config is public, shade.config.environment = "production" currently stores a raw string. Gateway later reads _config.environment and expects an Environment with .base_url, so this can crash during client setup. Route assignments through parse_environment.
Proposed fix
class Config:
"""Global SDK configuration."""
def __init__(self):
self.debug: bool = False
self._api_base: Optional[str] = None
self.timeout: float = DEFAULT_TIMEOUT
self.max_retries: int = DEFAULT_MAX_RETRIES
- self.environment: Environment = Environment.SANDBOX
+ self._environment: Environment = Environment.SANDBOX
+
+ `@property`
+ def environment(self) -> Environment:
+ return self._environment
+
+ `@environment.setter`
+ def environment(self, value: str | Environment) -> None:
+ self._environment = self.parse_environment(value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.environment: Environment = Environment.SANDBOX | |
| @property | |
| def api_base(self) -> Optional[str]: | |
| return self._api_base | |
| @api_base.setter | |
| def api_base(self, value: Optional[str]) -> None: | |
| self._api_base = value | |
| def parse_environment(self, value: str | Environment) -> Environment: | |
| if isinstance(value, Environment): | |
| return value | |
| if isinstance(value, str): | |
| try: | |
| return Environment(value.lower()) | |
| except ValueError: | |
| pass | |
| raise ValueError("Invalid environment. Valid options are: 'sandbox', 'production'") | |
| self._environment: Environment = Environment.SANDBOX | |
| `@property` | |
| def environment(self) -> Environment: | |
| return self._environment | |
| `@environment.setter` | |
| def environment(self, value: str | Environment) -> None: | |
| self._environment = self.parse_environment(value) | |
| `@property` | |
| def api_base(self) -> Optional[str]: | |
| return self._api_base | |
| `@api_base.setter` | |
| def api_base(self, value: Optional[str]) -> None: | |
| self._api_base = value | |
| def parse_environment(self, value: str | Environment) -> Environment: | |
| if isinstance(value, Environment): | |
| return value | |
| if isinstance(value, str): | |
| try: | |
| return Environment(value.lower()) | |
| except ValueError: | |
| pass | |
| raise ValueError("Invalid environment. Valid options are: 'sandbox', 'production'") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shade/config.py` around lines 16 - 34, The Config.environment assignment
currently bypasses parsing, so string values like "production" can be stored raw
and later break Gateway when it expects an Environment with base_url. Update the
Config class so environment is a parsed property like api_base, and route all
assignments through parse_environment while preserving the existing Environment
enum handling and validation.
Overview
This PR adds request/response debug logging to the Shade Python SDK so developers can troubleshoot integration issues with visibility into outgoing requests and incoming responses. It introduces
ShadeClientwith adebugflag, a globalconfig.debugsetting, and structuredlogging.DEBUGoutput under theshadelogger with sensitive values masked.Related Issue
Closes #13
Changes
🔍 Debug Logging System
[ADD]
src/shade/client.pyAdded
ShadeClientwithdebug: bool = Falseand an HTTPrequest()method.Emits debug logs for outgoing requests (method, URL, headers, body) and incoming responses (status, headers, body) when debug mode is enabled.
Supports both per-client
debug=Trueand globalconfig.debug = True.[ADD]
src/shade/_debug.pyAdded helpers to mask
Authorizationheader values (only last 4 characters visible).Added response body truncation at 2000 characters with a
[truncated]suffix.Added structured request/response logging via Python's
loggingmodule atDEBUGlevel under theshadelogger.[ADD]
src/shade/config.pyAdded global SDK configuration with
debug: bool = False.[MODIFY]
src/shade/__init__.pyExported
ShadeClientandconfigfrom the public API.🧪 Tests
[ADD]
tests/test_debug_logging.pyAdded tests for authorization header masking.
Added tests for response body truncation.
Added tests verifying debug logs are emitted when
debug=True.Added tests verifying no request/response content is logged when
debug=False.Added tests verifying global
config.debugenables logging.[MODIFY]
tests/test_gateway.pyRemoved unused
pytestimport.🔧 Dependencies
[MODIFY]
pyproject.tomlAdded
httpxas an HTTP client dependency forShadeClient.[MODIFY]
poetry.lockUpdated lockfile for the new
httpxdependency.Verification Results
debug=True, each request logs method, URL, and masked headersdebug=False(default), no request/response content is loggedlogging.DEBUGlevel under theshadelogger[truncated]Summary by CodeRabbit
ShadeClientwith configurable base URL, context-manager lifecycle, and debug logging.shade.config(including top-level export) via aConfigsingleton.Gatewaywith injected HTTP client/debug options, plusrequest(...),close(), and context-manager support.Authorizationand truncating logged request/response bodies.FEATURES.mdroadmap andISSUES.mdbacklog; addedhttpxdependency.