feat(skills): add error-handling skill#1559
feat(skills): add error-handling skill#1559Sayaka-Saeki wants to merge 3 commits intoaffaan-m:mainfrom
Conversation
…security scan, add error-handling skill Agent-Logs-Url: https://github.com/Sayaka-Saeki/everything-claude-code/sessions/60ee143f-8449-4422-a70b-7152976f8958 Co-authored-by: Sayaka-Saeki <94221513+Sayaka-Saeki@users.noreply.github.com>
… per review Agent-Logs-Url: https://github.com/Sayaka-Saeki/everything-claude-code/sessions/60ee143f-8449-4422-a70b-7152976f8958 Co-authored-by: Sayaka-Saeki <94221513+Sayaka-Saeki@users.noreply.github.com>
feat: improve skills – add error-handling skill, enhance e2e-testing and verification-loop
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37206fc83d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| -e 'API_KEY\s*=' \ | ||
| -e 'password\s*=\s*["'"'"']' \ | ||
| -e 'secret\s*=\s*["'"'"']' \ | ||
| . 2>/dev/null | grep -v 'node_modules' | grep -v '\.test\.' | grep -v '__tests__' | head -20 |
There was a problem hiding this comment.
The new secret-scan pipeline explicitly drops matches from \.test\. files and __tests__ paths, which means leaked credentials in test fixtures or integration tests will not be reported at all. Since test code is still committed and often shared in CI/public repos, this creates a real blind spot in the security gate and can allow exposed keys to pass verification unnoticed.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds a new
Confidence Score: 4/5Safe to merge after fixing the Go handler's err.Error() leak and the missing logger import in the Python example. One P1 finding (Go 404 handler leaks internal error details via err.Error() — contradicts the guide's own best-practice principle) warrants a 4/5. Remaining findings are P2 style/completeness suggestions. skills/error-handling/SKILL.md — Go handler and Python FastAPI example both need corrections before this guide is used as a reference. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[API Request] --> B{Try handler}
B -->|Success| C[Return 200 + data]
B -->|Throws| D{error instanceof AppError?}
D -->|Yes| E{error code}
E -->|NOT_FOUND| F["writeError(404, static_message)"]
E -->|UNAUTHORIZED| G["writeError(403, 'Access denied')"]
E -->|Other| H["writeError(statusCode, error.message)"]
D -->|No| I{ZodError?}
I -->|Yes| J["writeError(422, VALIDATION_ERROR)"]
I -->|No| K["log full details server-side"]
K --> L["writeError(500, generic message)"]
subgraph withRetry
M[attempt = 1] --> N{fn succeeds?}
N -->|Yes| O[return value]
N -->|No| P{last attempt or !retryIf?}
P -->|Yes| Q[throw error]
P -->|No| R[exponential backoff + jitter]
R --> S[attempt++]
S --> N
end
Reviews (1): Last reviewed commit: "Merge pull request #1 from Sayaka-Saeki/..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if err != nil { | ||
| switch { | ||
| case errors.Is(err, domain.ErrNotFound): | ||
| writeError(w, http.StatusNotFound, "not_found", err.Error()) |
There was a problem hiding this comment.
err.Error() on a fmt.Errorf-wrapped error produces the full chain (e.g. "user abc-123: not found"), exposing user IDs and internal path structure in the HTTP response body. This contradicts the guide's own Principle 3 ("User messages ≠ developer messages"). The ErrUnauthorized case already uses a static string — the ErrNotFound case should too.
| writeError(w, http.StatusNotFound, "not_found", err.Error()) | |
| writeError(w, http.StatusNotFound, "not_found", "The requested resource was not found") |
| let lastError: unknown | ||
|
|
There was a problem hiding this comment.
throw lastError throws undefined when maxAttempts is 0 or negative
The throw lastError after the loop is unreachable when maxAttempts >= 1 (the loop always throws on the final iteration). However, if a caller passes maxAttempts: 0, the loop never executes, lastError remains undefined, and the function throws undefined — a very unhelpful error. Adding a guard at the top makes this explicit and safe.
| let lastError: unknown | |
| if (maxAttempts <= 0) throw new Error('maxAttempts must be at least 1') | |
| let lastError: unknown |
| async def generic_error_handler(request: Request, exc: Exception) -> JSONResponse: | ||
| # Log full details, return generic message | ||
| logger.exception("Unexpected error", exc_info=exc) |
There was a problem hiding this comment.
logger is referenced but never imported or defined
logger.exception(...) in the generic exception handler references logger, but there is no import for it in the snippet. Readers copying this example will get a NameError at runtime. Add import logging and logger = logging.getLogger(__name__) (or import from structlog) at the top of the snippet.
| grep -rn --include="*.ts" --include="*.js" --include="*.tsx" \ | ||
| -e 'sk-[a-zA-Z0-9]' \ | ||
| -e 'api_key\s*=' \ | ||
| -e 'API_KEY\s*=' \ | ||
| -e 'password\s*=\s*["'"'"']' \ | ||
| -e 'secret\s*=\s*["'"'"']' \ | ||
| . 2>/dev/null | grep -v 'node_modules' | grep -v '\.test\.' | grep -v '__tests__' | head -20 |
There was a problem hiding this comment.
The --include flags cover only .ts, .js, and .tsx files. Secrets are frequently committed in .env, .yaml/.yml, .json, and .toml config files. Extending the scan to cover those formats would significantly reduce false negatives.
📝 WalkthroughWalkthroughThis PR enhances skill documentation by adding a "When to Activate" section to the E2E testing guide, introducing a comprehensive error-handling patterns document covering TypeScript/JavaScript, Python, and Go, and expanding the verification-loop security scanning with npm audit checks and regex-based hardcoded secret detection. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/error-handling/SKILL.md`:
- Around line 286-293: The NotFound branch currently returns raw err.Error() to
clients; change the errors.Is(err, domain.ErrNotFound) case to send a stable
user-facing message (e.g., "Resource not found") via writeError instead of
err.Error(), and log the full internal error server-side using slog.Error
(include "err" and any identifying context) so developers see details while
users only get the stable message; keep the writeError signature and other cases
unchanged and use the same pattern if other cases currently expose err.Error().
- Around line 115-116: The snippet uses z.ZodError but never imports z; add
import { z } from 'zod' alongside the existing NextRequest/NextResponse imports
so the handler can reference z.ZodError, and ensure AppError referenced in the
snippet is either defined in this file or imported (e.g., add an import for
AppError or paste its class definition) so the example is copy-paste safe.
In `@skills/verification-loop/SKILL.md`:
- Around line 76-82: The secret-scan grep command (the multi-line pattern
starting with grep -rn --include="*.ts" --include="*.js" --include="*.tsx" -e
'sk-[a-zA-Z0-9]' ...) should be updated to also include JSX files; add
--include="*.jsx" alongside the existing --include globs so files with the .jsx
extension are scanned by the sk-, api_key, API_KEY, password, and secret
patterns in the SKILL.md check.
🪄 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
Run ID: 8d288d6d-298e-4054-8003-24232c59298d
📒 Files selected for processing (3)
skills/e2e-testing/SKILL.mdskills/error-handling/SKILL.mdskills/verification-loop/SKILL.md
| import { NextRequest, NextResponse } from 'next/server' | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In current Zod TypeScript usage, what is the correct import syntax to use z.ZodError in a Next.js route handler snippet?
💡 Result:
The correct import syntax to use z.ZodError in current Zod (v4 as of 2026) TypeScript usage, including Next.js route handlers, is: typescript import { z } from "zod"; Then use it as z.ZodError for type checking: typescript if (error instanceof z.ZodError) { // Handle ZodError } This works in Next.js App Router route handlers (e.g., app/api/route.ts), as shown in official Zod v4 docs and multiple Next.js + Zod examples. No separate import for ZodError is needed; it's available on the z namespace after importing z.
Citations:
- 1: https://v4.zod.dev/basics?id=handling-errors
- 2: https://zod.dev/error-customization
- 3: https://dev.to/whoffagents/zod-v3-the-runtime-validation-guide-typescript-developers-actually-need-5ae9
- 4: https://dub.co/blog/zod-api-validation
- 5: https://readmedium.com/zod-and-next-js-api-routes-type-checked-apis-8e7fa2a10b6c
- 6: https://dev.to/whoffagents/zod-validation-patterns-for-nextjs-api-routes-forms-and-external-apis-1mdo
- 7: https://medium.com/@lucarestagno/how-to-validate-your-next-js-api-with-zod-and-typescript-51fa637c6231
🏁 Script executed:
cat -n skills/error-handling/SKILL.md | head -150 | tail -50Repository: affaan-m/everything-claude-code
Length of output: 1647
🏁 Script executed:
cat -n skills/error-handling/SKILL.md | sed -n '1,116p'Repository: affaan-m/everything-claude-code
Length of output: 4387
Add missing Zod import to make the Next.js error handler snippet copy-paste safe.
The code references z.ZodError on line 133 but z is not imported. Add import { z } from 'zod' to the imports at the top of the snippet. The AppError class on line 119 also needs either an import statement or definition in the file where this code is used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/error-handling/SKILL.md` around lines 115 - 116, The snippet uses
z.ZodError but never imports z; add import { z } from 'zod' alongside the
existing NextRequest/NextResponse imports so the handler can reference
z.ZodError, and ensure AppError referenced in the snippet is either defined in
this file or imported (e.g., add an import for AppError or paste its class
definition) so the example is copy-paste safe.
| case errors.Is(err, domain.ErrNotFound): | ||
| writeError(w, http.StatusNotFound, "not_found", err.Error()) | ||
| case errors.Is(err, domain.ErrUnauthorized): | ||
| writeError(w, http.StatusForbidden, "forbidden", "Access denied") | ||
| default: | ||
| slog.Error("unexpected error", "err", err) | ||
| writeError(w, http.StatusInternalServerError, "internal_error", "An unexpected error occurred") | ||
| } |
There was a problem hiding this comment.
Avoid returning raw err.Error() to clients in the Go example.
Line 287 leaks internal error text to users and conflicts with the guide’s own principle (user-friendly message vs developer detail). Return a stable user message and log full context server-side.
Suggested snippet fix
- case errors.Is(err, domain.ErrNotFound):
- writeError(w, http.StatusNotFound, "not_found", err.Error())
+ case errors.Is(err, domain.ErrNotFound):
+ writeError(w, http.StatusNotFound, "not_found", "User not found")📝 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.
| case errors.Is(err, domain.ErrNotFound): | |
| writeError(w, http.StatusNotFound, "not_found", err.Error()) | |
| case errors.Is(err, domain.ErrUnauthorized): | |
| writeError(w, http.StatusForbidden, "forbidden", "Access denied") | |
| default: | |
| slog.Error("unexpected error", "err", err) | |
| writeError(w, http.StatusInternalServerError, "internal_error", "An unexpected error occurred") | |
| } | |
| case errors.Is(err, domain.ErrNotFound): | |
| writeError(w, http.StatusNotFound, "not_found", "User not found") | |
| case errors.Is(err, domain.ErrUnauthorized): | |
| writeError(w, http.StatusForbidden, "forbidden", "Access denied") | |
| default: | |
| slog.Error("unexpected error", "err", err) | |
| writeError(w, http.StatusInternalServerError, "internal_error", "An unexpected error occurred") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/error-handling/SKILL.md` around lines 286 - 293, The NotFound branch
currently returns raw err.Error() to clients; change the errors.Is(err,
domain.ErrNotFound) case to send a stable user-facing message (e.g., "Resource
not found") via writeError instead of err.Error(), and log the full internal
error server-side using slog.Error (include "err" and any identifying context)
so developers see details while users only get the stable message; keep the
writeError signature and other cases unchanged and use the same pattern if other
cases currently expose err.Error().
| grep -rn --include="*.ts" --include="*.js" --include="*.tsx" \ | ||
| -e 'sk-[a-zA-Z0-9]' \ | ||
| -e 'api_key\s*=' \ | ||
| -e 'API_KEY\s*=' \ | ||
| -e 'password\s*=\s*["'"'"']' \ | ||
| -e 'secret\s*=\s*["'"'"']' \ | ||
| . 2>/dev/null | grep -v 'node_modules' | grep -v '\.test\.' | grep -v '__tests__' | head -20 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether .jsx files exist and could be missed by current secret scan.
# Expected: If count > 0, include .jsx in the grep include list.
echo "JS/TS source file counts:"
fd -e ts -e tsx -e js -e jsx . | sed 's|^\./||' | awk -F. '
{ext=$NF; c[ext]++}
END {for (k in c) print k ": " c[k]}
' | sort
echo
echo "Sample .jsx files (if any):"
fd -e jsx . | head -20Repository: affaan-m/everything-claude-code
Length of output: 150
🏁 Script executed:
# Verify the file exists and check the exact content at lines 76-82
if [ -f "skills/verification-loop/SKILL.md" ]; then
echo "=== File found. Content at lines 76-82 ==="
sed -n '76,82p' "skills/verification-loop/SKILL.md"
else
echo "File not found at skills/verification-loop/SKILL.md"
# Try to find it
fd "SKILL.md" skills/
fiRepository: affaan-m/everything-claude-code
Length of output: 405
Add .jsx file extension to secret scan for future-proofing.
While no .jsx files currently exist in the repository, the secret scan at lines 76-82 should include --include="*.jsx" to prevent hardcoded secrets in JSX files if they are added later. This ensures consistent secret detection across all JavaScript/TypeScript source file types.
🔧 Suggested patch
-grep -rn --include="*.ts" --include="*.js" --include="*.tsx" \
+grep -rn --include="*.ts" --include="*.js" --include="*.tsx" --include="*.jsx" \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/verification-loop/SKILL.md` around lines 76 - 82, The secret-scan grep
command (the multi-line pattern starting with grep -rn --include="*.ts"
--include="*.js" --include="*.tsx" -e 'sk-[a-zA-Z0-9]' ...) should be updated to
also include JSX files; add --include="*.jsx" alongside the existing --include
globs so files with the .jsx extension are scanned by the sk-, api_key, API_KEY,
password, and secret patterns in the SKILL.md check.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/verification-loop/SKILL.md">
<violation number="1" location="skills/verification-loop/SKILL.md:82">
P2: Secret scan suppresses test-file hits, which can hide real committed secrets from manual review.</violation>
</file>
<file name="skills/error-handling/SKILL.md">
<violation number="1" location="skills/error-handling/SKILL.md:133">
P2: API error-handler example references `z.ZodError` without importing `z` from `zod`, making the snippet fail when used as-is.</violation>
<violation number="2" location="skills/error-handling/SKILL.md:246">
P2: FastAPI exception-handler example uses `logger` without defining/importing it, making the documented error path fail with `NameError`.</violation>
<violation number="3" location="skills/error-handling/SKILL.md:286">
P2: Go example is internally inconsistent by qualifying same-package sentinel errors with `domain.`, which makes the snippet invalid/misleading as copyable code.</violation>
<violation number="4" location="skills/error-handling/SKILL.md:287">
P1: Go 404 handler passes `err.Error()` directly to the client, which leaks internal error context (e.g., wrapped messages like `"user abc-123: not found"`) in the HTTP response. This contradicts Principle 3 stated earlier in this same guide ("User messages ≠ developer messages"). Use a static, user-friendly string here, consistent with how the `ErrUnauthorized` case already uses `"Access denied"`.</violation>
<violation number="5" location="skills/error-handling/SKILL.md:336">
P2: `withRetry` does not validate `maxAttempts`, so non-positive values skip all attempts and end by throwing `undefined`.</violation>
<violation number="6" location="skills/error-handling/SKILL.md:340">
P2: Retry example is behaviorally incorrect: it does not classify HTTP 4xx/5xx because fetch does not throw on non-2xx responses.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if err != nil { | ||
| switch { | ||
| case errors.Is(err, domain.ErrNotFound): | ||
| writeError(w, http.StatusNotFound, "not_found", err.Error()) |
There was a problem hiding this comment.
P1: Go 404 handler passes err.Error() directly to the client, which leaks internal error context (e.g., wrapped messages like "user abc-123: not found") in the HTTP response. This contradicts Principle 3 stated earlier in this same guide ("User messages ≠ developer messages"). Use a static, user-friendly string here, consistent with how the ErrUnauthorized case already uses "Access denied".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/error-handling/SKILL.md, line 287:
<comment>Go 404 handler passes `err.Error()` directly to the client, which leaks internal error context (e.g., wrapped messages like `"user abc-123: not found"`) in the HTTP response. This contradicts Principle 3 stated earlier in this same guide ("User messages ≠ developer messages"). Use a static, user-friendly string here, consistent with how the `ErrUnauthorized` case already uses `"Access denied"`.</comment>
<file context>
@@ -0,0 +1,376 @@
+ if err != nil {
+ switch {
+ case errors.Is(err, domain.ErrNotFound):
+ writeError(w, http.StatusNotFound, "not_found", err.Error())
+ case errors.Is(err, domain.ErrUnauthorized):
+ writeError(w, http.StatusForbidden, "forbidden", "Access denied")
</file context>
| -e 'API_KEY\s*=' \ | ||
| -e 'password\s*=\s*["'"'"']' \ | ||
| -e 'secret\s*=\s*["'"'"']' \ | ||
| . 2>/dev/null | grep -v 'node_modules' | grep -v '\.test\.' | grep -v '__tests__' | head -20 |
There was a problem hiding this comment.
P2: Secret scan suppresses test-file hits, which can hide real committed secrets from manual review.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/verification-loop/SKILL.md, line 82:
<comment>Secret scan suppresses test-file hits, which can hide real committed secrets from manual review.</comment>
<file context>
@@ -65,14 +65,28 @@ Report:
+ -e 'API_KEY\s*=' \
+ -e 'password\s*=\s*["'"'"']' \
+ -e 'secret\s*=\s*["'"'"']' \
+ . 2>/dev/null | grep -v 'node_modules' | grep -v '\.test\.' | grep -v '__tests__' | head -20
+
+# Check for leftover debug statements
</file context>
| . 2>/dev/null | grep -v 'node_modules' | grep -v '\.test\.' | grep -v '__tests__' | head -20 | |
| . 2>/dev/null | grep -v 'node_modules' | head -20 |
| user, err := h.service.GetUser(r.Context(), chi.URLParam(r, "id")) | ||
| if err != nil { | ||
| switch { | ||
| case errors.Is(err, domain.ErrNotFound): |
There was a problem hiding this comment.
P2: Go example is internally inconsistent by qualifying same-package sentinel errors with domain., which makes the snippet invalid/misleading as copyable code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/error-handling/SKILL.md, line 286:
<comment>Go example is internally inconsistent by qualifying same-package sentinel errors with `domain.`, which makes the snippet invalid/misleading as copyable code.</comment>
<file context>
@@ -0,0 +1,376 @@
+ user, err := h.service.GetUser(r.Context(), chi.URLParam(r, "id"))
+ if err != nil {
+ switch {
+ case errors.Is(err, domain.ErrNotFound):
+ writeError(w, http.StatusNotFound, "not_found", err.Error())
+ case errors.Is(err, domain.ErrUnauthorized):
</file context>
| } | ||
| } | ||
|
|
||
| throw lastError |
There was a problem hiding this comment.
P2: withRetry does not validate maxAttempts, so non-positive values skip all attempts and end by throwing undefined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/error-handling/SKILL.md, line 336:
<comment>`withRetry` does not validate `maxAttempts`, so non-positive values skip all attempts and end by throwing `undefined`.</comment>
<file context>
@@ -0,0 +1,376 @@
+ }
+ }
+
+ throw lastError
+}
+
</file context>
| } | ||
|
|
||
| // Usage: retry transient network errors, not 4xx | ||
| const data = await withRetry(() => fetch('/api/data').then(r => r.json()), { |
There was a problem hiding this comment.
P2: Retry example is behaviorally incorrect: it does not classify HTTP 4xx/5xx because fetch does not throw on non-2xx responses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/error-handling/SKILL.md, line 340:
<comment>Retry example is behaviorally incorrect: it does not classify HTTP 4xx/5xx because fetch does not throw on non-2xx responses.</comment>
<file context>
@@ -0,0 +1,376 @@
+}
+
+// Usage: retry transient network errors, not 4xx
+const data = await withRetry(() => fetch('/api/data').then(r => r.json()), {
+ maxAttempts: 3,
+ retryIf: (error) => !(error instanceof AppError && error.statusCode < 500),
</file context>
| @app.exception_handler(Exception) | ||
| async def generic_error_handler(request: Request, exc: Exception) -> JSONResponse: | ||
| # Log full details, return generic message | ||
| logger.exception("Unexpected error", exc_info=exc) |
There was a problem hiding this comment.
P2: FastAPI exception-handler example uses logger without defining/importing it, making the documented error path fail with NameError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/error-handling/SKILL.md, line 246:
<comment>FastAPI exception-handler example uses `logger` without defining/importing it, making the documented error path fail with `NameError`.</comment>
<file context>
@@ -0,0 +1,376 @@
+@app.exception_handler(Exception)
+async def generic_error_handler(request: Request, exc: Exception) -> JSONResponse:
+ # Log full details, return generic message
+ logger.exception("Unexpected error", exc_info=exc)
+ return JSONResponse(
+ status_code=500,
</file context>
| } | ||
|
|
||
| // Zod validation error | ||
| if (error instanceof z.ZodError) { |
There was a problem hiding this comment.
P2: API error-handler example references z.ZodError without importing z from zod, making the snippet fail when used as-is.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/error-handling/SKILL.md, line 133:
<comment>API error-handler example references `z.ZodError` without importing `z` from `zod`, making the snippet fail when used as-is.</comment>
<file context>
@@ -0,0 +1,376 @@
+ }
+
+ // Zod validation error
+ if (error instanceof z.ZodError) {
+ return NextResponse.json(
+ {
</file context>
|
Thanks for the PR. This has been idle for a while and is not merge-ready against the moving main branch, so I am closing it to keep the queue tractable. Reopen is welcome with a current rebase and focused scope; maintainers may also port the useful parts into a fresh PR where appropriate. |
This pull request introduces a new skill guide for robust error handling across TypeScript, Python, and Go, and improves documentation for E2E testing and verification workflows. The main focus is on providing actionable, language-agnostic patterns and checklists for error handling, as well as strengthening security and test documentation.
New error handling patterns and improvements:
error-handlingskill guide covering typed errors, error boundaries, retries, circuit breakers, and user-facing messages in TypeScript, Python, and Go, including code examples and a pre-merge checklist.Documentation improvements for E2E testing:
skills/e2e-testing/SKILL.mdto clarify scenarios for using Playwright E2E patterns.Security and verification workflow enhancements:
skills/verification-loop/SKILL.mdto include dependency vulnerability checks (npm audit), improved secret detection patterns, and clarified manual review steps. Also, emphasized flagging and rotating hardcoded secrets before merging.Summary by cubic
Adds a cross‑language error‑handling skill with actionable patterns and a merge checklist, and improves E2E and verification docs for clearer activation and stronger security scans.
skills/error-handling/SKILL.md: typed errors, result pattern, API error handlers, React error boundary, FastAPI handlers, Go sentinels/wrapping, retry with backoff, user‑friendly messages, and a pre‑merge checklist.skills/e2e-testing/SKILL.md: new “When to Activate” section for Playwright use cases (critical flows, CI, flake triage, cross‑browser).skills/verification-loop/SKILL.md: expanded Security Scan withnpm audit/pnpm audit/yarn npm audit, broader secret detection patterns, guidance to flag and rotate hardcoded secrets, and a check for leftoverconsole.logstatements.Written for commit 37206fc. Summary will update on new commits.
Summary by CodeRabbit