add sdk debug mode #16306
add sdk debug mode #16306
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe changes introduce a new debugging utility and environment variable to the SDK, enabling conditional logging of sanitized debug information for API requests. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SDK (BaseClient)
participant API
User->>SDK (BaseClient): makeRequest(url, options)
SDK (BaseClient)->>API: Send HTTP request
API-->>SDK (BaseClient): Respond with status, headers, body
SDK (BaseClient)->>SDK (BaseClient): Read response as raw text
alt PD_SDK_DEBUG is true
SDK (BaseClient)->>SDK (BaseClient): Sanitize and log debug info using DEBUG()
end
alt Response is OK and JSON
SDK (BaseClient)->>User: Return parsed JSON
else Response is OK and not JSON
SDK (BaseClient)->>User: Return raw text
else Response is not OK
SDK (BaseClient)->>User: Throw error with raw body
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/sdk/src/shared/index.tsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sdk/src/shared/index.ts (2)
892-901: Good implementation of conditional debug logging.The
DEBUGfunction:
- Checks if the
PD_SDK_DEBUGenvironment variable is set to "true"- Sanitizes all arguments to prevent leaking sensitive information
- Prefixes logged messages with
[PD_SDK_DEBUG]for easy identificationConsider adding documentation about this feature in user-facing documentation to help developers troubleshoot SDK issues.
export function DEBUG(...args: any[]) { + // Only logs when PD_SDK_DEBUG=true environment variable is set + // Sanitizes sensitive information before logging if ( typeof process !== "undefined" && typeof process.env !== "undefined" && process.env.PD_SDK_DEBUG === "true" ) { const safeArgs = args.map((arg) => sanitize(arg)); console.log("[PD_SDK_DEBUG]", ...safeArgs); } }
1025-1034: Improved JSON parsing with proper error handling.The implementation now:
- Attempts to parse JSON only if the content type indicates JSON
- Uses a try-catch block to handle JSON parsing errors gracefully
- Logs debug information when parsing fails
- Falls back to returning raw text when JSON parsing fails
This makes the API client more robust when dealing with unexpected or malformed responses.
Consider using the optional chaining operator as suggested by the static analysis tool:
-if (contentType && contentType.includes("application/json")) { +if (contentType?.includes("application/json")) { try { const json = JSON.parse(rawBody); return json as T; } catch (err) { DEBUG("Couldn't parse json, falling back to raw", err) } }🧰 Tools
🪛 Biome (1.9.4)
[error] 1025-1025: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json(1 hunks)packages/sdk/CHANGELOG.md(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/shared/index.ts(2 hunks)packages/sdk/src/shared/shims.d.ts(1 hunks)packages/sdk/tsconfig.browser.json(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/sdk/src/shared/index.ts
[error] 1025-1025: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (8)
package.json (1)
44-44: Reordering of devDependency looks good.The dependency
@next/eslint-plugin-nexthas been reordered within thedevDependenciessection without changing its version. This is likely due to automatic sorting or code formatting.packages/sdk/src/shared/shims.d.ts (1)
1-5: Good addition of TypeScript declaration for environment variable access.This TypeScript declaration provides proper type information for accessing environment variables via
process.envin browser environments whereprocessmight be undefined. This supports the new debugging feature that needs to check for thePD_SDK_DEBUGenvironment variable.packages/sdk/tsconfig.browser.json (1)
17-17: Correctly included declaration file in TypeScript configuration.The new
src/shared/shims.d.tsdeclaration file has been properly added to the TypeScript compiler's include list, ensuring that the browser build has access to theprocesstype declarations.packages/sdk/src/shared/index.ts (3)
850-859: Good list of sensitive keys for redaction.This comprehensive list of sensitive key patterns ensures that sensitive information like tokens, passwords, and keys will be properly redacted in debug output.
862-889: Well-implemented sanitization function with circular reference handling.The
sanitizefunction effectively:
- Handles null/undefined values
- Detects and marks circular references to prevent infinite recursion
- Processes arrays recursively
- Redacts values for sensitive keys
- Recursively processes nested objects
This implementation carefully handles edge cases that could occur in complex API requests and responses.
1017-1023: Fixed response body handling to prevent multiple consumption issues.This change addresses a critical issue where attempting to read a response body multiple times could fail. By:
- Reading the raw body text once and storing it
- Including this raw body in error messages for better debugging
- Logging sanitized debug information about the request and response
This improves error reporting and fixes potential runtime errors that could occur when trying to access response data multiple times.
packages/sdk/package.json (1)
4-4: Version bump is appropriate and aligns with new feature addition.The version update to 1.5.1 is correct for introducing the debug mode feature.
packages/sdk/CHANGELOG.md (1)
5-12: Changelog entry is clear and accurate.The new entry documents the debug feature and its risks well.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sdk/src/shared/index.ts (2)
897-906: Safe implementation of debug logging.The DEBUG function correctly:
- Checks for the existence of the process object and environment variables
- Only logs when explicitly enabled via PD_SDK_DEBUG environment variable
- Applies sanitization to all arguments before logging
- Provides a clear prefix for easy identification of debug logs
One improvement could be handling variations of "true" like "1" or "yes".
- if ( - typeof process !== "undefined" && - typeof process.env !== "undefined" && - process.env.PD_SDK_DEBUG === "true" - ) { + if ( + typeof process !== "undefined" && + typeof process.env !== "undefined" && + ["true", "1", "yes"].includes(process.env.PD_SDK_DEBUG?.toLowerCase() || "") + ) {
1029-1040: Safer response handling with fallback mechanism.This approach addresses a common Fetch API issue where response bodies can only be consumed once. The implementation:
- Captures the raw text first
- Attempts to parse JSON when appropriate
- Falls back to the raw text with proper debug logging if parsing fails
Based on the static analysis hint, consider using optional chaining for the Content-Type header check:
- const contentType = response.headers.get("Content-Type"); - if (contentType && contentType.includes("application/json")) { + const contentType = response.headers.get("Content-Type"); + if (contentType?.includes("application/json")) {🧰 Tools
🪛 Biome (1.9.4)
[error] 1030-1030: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/sdk/CHANGELOG.md(1 hunks)packages/sdk/src/shared/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk/CHANGELOG.md
🧰 Additional context used
🪛 Biome (1.9.4)
packages/sdk/src/shared/index.ts
[error] 1030-1030: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
packages/sdk/src/shared/index.ts (4)
855-864: Good selection of sensitive keys for redaction.The array of sensitive keys covers common patterns for credentials and private information, which will help ensure sensitive data is not accidentally logged.
867-894: Well-implemented sanitization function with circular reference handling.The sanitize function effectively:
- Recursively processes objects and arrays
- Detects and handles circular references with WeakSet
- Performs case-insensitive matching of sensitive keys
- Properly redacts sensitive values
This approach ensures debug logs won't leak credentials or sensitive data.
1022-1026: Improved error handling with response body capture.Capturing the raw body text before checking response status improves error messages by including the response content, which is essential for debugging API issues.
1028-1028: Effective debug logging of request and response details.The DEBUG function is used appropriately to log all relevant information for troubleshooting: status code, request URL, options, and the raw response body.
| // Attempt to parse JSON, fall back to raw text if it fails | ||
| DEBUG(response.status, url.toString(), requestOptions, rawBody) | ||
| const contentType = response.headers.get("Content-Type"); | ||
| if (contentType && contentType.includes("application/json")) { |
There was a problem hiding this comment.
This should have used optional chaining I guess.
2 participants
WHY
Summary by CodeRabbit