-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Improve error messages with actionable guidance #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Extract and display error details from API response body - Add specific guidance for common errors: - Nested archives: suggest using .gitattributes export-ignore - Size limits: suggest excluding large files - Rate limits: suggest waiting before retry - Handle 401, 413, 429, and 500 status codes with helpful messages
WalkthroughUpdated Changes
Sequence Diagram(s)sequenceDiagram
participant Action as Action
participant API as External API
participant Handler as Error Handler (src/index.ts)
participant Logger as GitHub Actions core
Action->>API: make request
API-->>Action: responds with error (status, body, headers)
Action->>Handler: catch error
Handler->>Handler: safeSerialize & redactSensitive(error/response)
Handler->>Handler: determine status & apiMessage from multiple fields
Handler->>Logger: core.debug/core.info structured logs (message, name, status, body, headers)
Handler->>Logger: core.setFailed(errorMessage + optional helpText)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/index.ts`:
- Around line 123-148: The current error-logging block uses core.info and direct
JSON.stringify calls that can leak secrets (Authorization/Cookie) and throw on
circular/BigInt values; change logging to use core.debug for sensitive details,
replace direct JSON.stringify calls with a safeSerialize helper (that catches
stringify errors and returns a deterministic placeholder like "[unserializable]"
or truncates large values), and redact sensitive header/body fields (e.g.,
Authorization, Cookie, set-cookie, access_token) before serializing; ensure all
uses in this block (references: core.info, JSON.stringify, error,
error.response, error.response.headers, error.response.data, error.body,
error.cause) are wrapped so they never throw and never log raw secret values.
🧹 Nitpick comments (1)
src/index.ts (1)
178-187: Make the 500‑error heuristics case‑insensitive.API messages can vary in casing; lowercasing once avoids missing matches.
♻️ Small tweak
- } else if (status === 500) { + } else if (status === 500) { errorMessage = apiMessage || 'Internal server error'; // Check for common issues and provide guidance - if (apiMessage.includes('Nested archives')) { + const msg = apiMessage.toLowerCase(); + if (msg.includes('nested archives')) { helpText = 'Your repository contains nested archive files (.zip, .tar, etc.). ' + 'Add them to .gitattributes with "export-ignore" to exclude from analysis. ' + 'Example: tests/fixtures/*.zip export-ignore'; - } else if (apiMessage.includes('exceeds maximum')) { + } else if (msg.includes('exceeds maximum')) { helpText = 'Your repository or a file within it exceeds size limits. ' + 'Consider excluding large files using .gitattributes with "export-ignore".'; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (1)
src/index.ts
🔇 Additional comments (1)
src/index.ts (1)
150-199: Nice, resilient message extraction chain.The multi-source
apiMessagefallback plus status-based messaging is clear and should make errors much more actionable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Add safeSerialize helper to handle circular refs, BigInt, and truncate large values - Add redactSensitive helper to redact Authorization, Cookie, token fields - Use core.debug for sensitive details (headers, cause) - Use core.info for safe error summaries (status, message, data) - Wrap all serialization in try/catch to prevent throws - Add SENSITIVE_KEYS set for consistent redaction
Summary
.gitattributes export-ignoreContext
This addresses the issue where users see generic "API error (500)" without understanding the root cause. The most common cause is nested archive files (
.zip,.tar, etc.) triggering the API's zip bomb protection.Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.