fix: preserve typed API error prototypes#992
Conversation
👷 Deploy request for docmagic-muneer pending review.Visit the deploys page to approve it
|
👷 Deploy request for docmagic1 pending review.Visit the deploys page to approve it
|
|
@saurabhhhcodes is attempting to deploy a commit to the muneerali199's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds SEO infrastructure (robots and sitemap metadata routes), updates Jest test configuration with global polyfills for missing APIs, and improves API error handling with corrected prototype chains and explicit HTTP headers. ChangesSEO metadata and infrastructure improvements
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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
ESLint install timed out. The project may have too many dependencies for the sandbox. 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: 1
🧹 Nitpick comments (2)
jest.config.mjs (1)
1-8: ⚡ Quick winRemove redundant crypto polyfill from config file.
The crypto polyfill here duplicates the one in
jest.polyfills.cjs(line 11). SincesetupFiles(which loads the polyfill module) runs early enough for all tests, this top-level polyfill in the config file is unnecessary. Polyfills belong in the dedicatedjest.polyfills.cjsfile for cleaner separation of concerns.♻️ Proposed fix to remove the duplicate polyfill
-import { webcrypto } from 'node:crypto'; - -if (!globalThis.crypto) { - Object.defineProperty(globalThis, 'crypto', { - value: webcrypto, - configurable: true, - }); -} - const { default: nextJest } = await import('next/jest.js');🤖 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 `@jest.config.mjs` around lines 1 - 8, The top-level crypto polyfill in jest.config.mjs (the globalThis.crypto definition using node:crypto webcrypto) is redundant because the same polyfill is provided in jest.polyfills.cjs and loaded via setupFiles; remove the entire polyfill block from jest.config.mjs (the import of webcrypto and the Object.defineProperty call that sets globalThis.crypto) so that the sole polyfill lives in jest.polyfills.cjs and setupFiles continues to load it.jest.polyfills.cjs (1)
16-23:undici.Fileis included in undici v6.24.0 (File API available)
undiciv6.24.0 ships the Web File API, soundici.Fileshould exist andglobalThis.File = undici.Fileinjest.polyfills.cjswon’t be undefined for that version. Since these Web-API implementations were removed inundiciv7, keep the dependency on the v6 line (or otherwise guard this assignment) ifundicican be upgraded.🤖 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 `@jest.polyfills.cjs` around lines 16 - 23, The current polyfill unconditionally assigns undici.File to globalThis.File which will be undefined if undici v7+ is used; update the assignment in jest.polyfills.cjs to guard the export (e.g., check if undici.File is present before setting globalThis.File) or enforce using the undici v6 line in package.json; specifically, change the globalThis.File = undici.File line to only run when typeof undici.File !== 'undefined' (while leaving other fetch/Headers/Request/Response/FormData assignments unchanged) so tests remain compatible across undici versions.
🤖 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 `@lib/seo-routes.ts`:
- Around line 16-30: privateRobotsPaths currently lists some routes with
trailing slashes (e.g., '/documents/', '/settings/', '/subscription/') which
won’t block the base pages ('/documents', '/settings', '/subscription') in
robots prefix matching; update the privateRobotsPaths constant to include both
the base path and the trailing-slash variant for each of those entries or
normalize by removing trailing slashes so that '/documents', '/settings',
'/subscription' are present alongside or instead of the '/.../' forms to ensure
both '/documents' and '/documents/*' are disallowed.
---
Nitpick comments:
In `@jest.config.mjs`:
- Around line 1-8: The top-level crypto polyfill in jest.config.mjs (the
globalThis.crypto definition using node:crypto webcrypto) is redundant because
the same polyfill is provided in jest.polyfills.cjs and loaded via setupFiles;
remove the entire polyfill block from jest.config.mjs (the import of webcrypto
and the Object.defineProperty call that sets globalThis.crypto) so that the sole
polyfill lives in jest.polyfills.cjs and setupFiles continues to load it.
In `@jest.polyfills.cjs`:
- Around line 16-23: The current polyfill unconditionally assigns undici.File to
globalThis.File which will be undefined if undici v7+ is used; update the
assignment in jest.polyfills.cjs to guard the export (e.g., check if undici.File
is present before setting globalThis.File) or enforce using the undici v6 line
in package.json; specifically, change the globalThis.File = undici.File line to
only run when typeof undici.File !== 'undefined' (while leaving other
fetch/Headers/Request/Response/FormData assignments unchanged) so tests remain
compatible across undici versions.
🪄 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: 6aee590e-90f1-48b2-9d2b-f79c7aa1244e
📒 Files selected for processing (6)
app/robots.tsapp/sitemap.tsjest.config.mjsjest.polyfills.cjslib/api-handler.tslib/seo-routes.ts
| export const privateRobotsPaths = [ | ||
| '/api/', | ||
| '/auth/', | ||
| '/dashboard/', | ||
| '/diagnostic', | ||
| '/documents/', | ||
| '/editor', | ||
| '/profile', | ||
| '/resume-builder', | ||
| '/resume-builder-simple', | ||
| '/resume-editor', | ||
| '/settings/', | ||
| '/subscription/', | ||
| '/test-ats', | ||
| ] as const; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if any of the paths without trailing slashes have sub-routes in the codebase
fd -t f -e ts -e tsx -e js -e jsx | rg -l 'diagnostic|^editor(?!/)|profile|resume-builder|resume-editor|test-ats' | head -20Repository: Muneerali199/Draftdeckai
Length of output: 359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== lib/seo-routes.ts (relevant section) ==="
if [ -f lib/seo-routes.ts ]; then
nl -ba lib/seo-routes.ts | sed -n '1,140p'
else
echo "File lib/seo-routes.ts not found"
fi
echo
echo "=== Usage of privateRobotsPaths / robots ==="
rg -n "privateRobotsPaths|robots\.txt|Disallow|Allow" lib src . || true
echo
echo "=== Re-run sub-route search (PCRE2) for paths without trailing slashes ==="
# Look for occurrences of the no-trailing-slash path followed by a slash (subpath), e.g. /editor/<...>
# Also catch route patterns like "/editor/*" or "/editor/:id"
fd -t f -e ts -e tsx -e js -e jsx . \
| xargs -r rg -n --pcre2 -S "(/\s*(diagnostic|editor|profile|resume-builder|resume-editor|test-ats)(/[^\"'\\s]*)?|['\"/(](diagnostic|editor|profile|resume-builder|resume-editor|test-ats)[/'\"(])" \
--no-heading --color never \
| head -80 || trueRepository: Muneerali199/Draftdeckai
Length of output: 174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check file existence ==="
ls -la lib/seo-routes.ts 2>/dev/null || echo "Missing: lib/seo-routes.ts"
echo
echo "=== lib/seo-routes.ts (first 220 lines with line numbers) ==="
if [ -f lib/seo-routes.ts ]; then
cat -n lib/seo-routes.ts | sed -n '1,220p'
fi
echo
echo "=== Search for privateRobotsPaths / robots.txt generation ==="
rg -n "privateRobotsPaths|robots\.txt|Disallow:|Disallow |Allow:|Allow " lib src . || true
echo
echo "=== Sub-route existence for no-trailing-slash entries ==="
# These are the entries in the snippet that lack trailing slashes:
# /diagnostic, /editor, /profile, /resume-builder, /resume-builder-simple, /resume-editor, /test-ats
# Check for occurrences of "/<path>/" (subpath usage) and "/<path>?"/"=<...>" patterns as well.
fd -t f -e ts -e tsx -e js -e jsx . \
| xargs -r rg -n -S --pcre2 \
'["'\'']/(diagnostic|editor|profile|resume-builder|resume-builder-simple|resume-editor|test-ats)/[^"'\''\s]+' \
--no-heading --color never \
| head -120 || trueRepository: Muneerali199/Draftdeckai
Length of output: 3526
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== app/robots.ts ==="
ls -la app/robots.ts
sed -n '1,160p' app/robots.ts
echo
echo "=== Route structure under app/ for relevant prefixes ==="
# List directories at depth <= 2 and files at depth <= 3, then filter to relevant route segments.
# Handles Next.js app router segments like editor, profile, diagnostic, etc.
find app -maxdepth 3 -type d -print 2>/dev/null | sed 's|^\./||' | rg -n "/(api|auth|dashboard|diagnostic|documents|editor|profile|resume-builder|resume-builder-simple|resume-editor|settings|subscription|test-ats)(/|$)" || true
echo
echo "=== Files under those prefixes (look for page/route/layout) ==="
find app -maxdepth 4 -type f -print 2>/dev/null | sed 's|^\./||' | rg -n "(api|auth|dashboard|diagnostic|documents|editor|profile|resume-builder|resume-builder-simple|resume-editor|settings|subscription|test-ats)/.*(page\.|route\.|layout\.|loading\.|error\.|middleware\.|server\.|ts)$" || trueRepository: Muneerali199/Draftdeckai
Length of output: 6040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
prefixes=(api auth dashboard diagnostic documents editor profile resume-builder resume-builder-simple resume-editor settings subscription test-ats)
echo "=== Check for base pages at each prefix (page.* directly under prefix folder) ==="
for p in "${prefixes[@]}"; do
echo "--- app/$p/*page.* (maxdepth 1) ---"
find "app/$p" -maxdepth 1 -type f \( -name 'page.*' -o -name 'route.*' -o -name 'layout.*' \) 2>/dev/null | sed 's|^app/||' || true
done
echo
echo "=== Check for root-level page.tsx specifically (exact /<prefix>) ==="
for p in "${prefixes[@]}"; do
echo "--- app/$p/page.* ---"
find "app/$p" -maxdepth 1 -type f -name 'page.tsx' 2>/dev/null | sed 's|^app/||' || true
done
echo
echo "=== List files under each prefix (small sample, depth<=2) ==="
for p in "${prefixes[@]}"; do
echo "--- sample app/$p (maxdepth 2, type f) ---"
find "app/$p" -maxdepth 2 -type f 2>/dev/null | sed 's|^app/||' | head -60 || true
doneRepository: Muneerali199/Draftdeckai
Length of output: 3460
Fix robots.txt Disallow rules to block base routes too
privateRobotsPaths includes trailing-slash disallows (/documents/, /settings/, /subscription/), but those prefixes have base pages without the slash (app/documents/page.tsx, app/settings/page.tsx, app/subscription/page.tsx). With robots prefix matching, disallowing /documents/ won’t block /documents (same for the others). Disallow both the base and the subpaths consistently (e.g., add /documents, /settings, /subscription entries or remove trailing slashes for these).
🤖 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 `@lib/seo-routes.ts` around lines 16 - 30, privateRobotsPaths currently lists
some routes with trailing slashes (e.g., '/documents/', '/settings/',
'/subscription/') which won’t block the base pages ('/documents', '/settings',
'/subscription') in robots prefix matching; update the privateRobotsPaths
constant to include both the base path and the trailing-slash variant for each
of those entries or normalize by removing trailing slashes so that '/documents',
'/settings', '/subscription' are present alongside or instead of the '/.../'
forms to ensure both '/documents' and '/documents/*' are disallowed.
Muneerali199
added
type:bug
|
@SaurabhhCodes please rebase on latest main to resolve conflicts |
2 participants
Summary\n- keep API error prototypes intact through the handler path\n- preserve typed errors for downstream consumers\n\n## Validation\n- reviewed the one-line behavior change\n- no broad runtime changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests