fix: catch HttpError in getRoutedUrl#23844
Conversation
WalkthroughImports in packages/features/routing-forms/lib/getRoutedUrl.ts were changed to remove Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
keithwillcode
added
core
| crmContactOwnerRecordType = result.crmContactOwnerRecordType; | ||
| crmAppSlug = result.crmAppSlug; | ||
| } catch (e) { | ||
| if (e instanceof TRPCError) { |
There was a problem hiding this comment.
There was a problem hiding this comment.
I think it might be safer to keep both, considering that if some fn somewhere starts throwing TRPCError or maybe throwing it currently.
![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)
packages/features/routing-forms/lib/getRoutedUrl.ts (2)
174-183: Back-compat and resilience: also handle legacy TRPCError shape and keep original error contextCatching only
HttpErrorrisks regressions during migration if upstream still throws TRPCError, and wrapping the error loses stack/context.Apply:
- } catch (e) { - if (e instanceof HttpError) { + } catch (e: unknown) { + const isLegacyTRPC = typeof e === "object" && e !== null && (e as any).name === "TRPCError"; + if (e instanceof HttpError || isLegacyTRPC) { + const msg = (e as any)?.message ?? "Request failed"; return { props: { ...pageProps, form: serializableForm, message: null, - errorMessage: e.message, + errorMessage: msg, }, }; } - - log.error("Error handling the response", safeStringify(e)); - throw new Error("Error handling the response"); + + log.error("Error handling the response", safeStringify(e)); + throw e as Error; }
180-181: Confirm user-facing safety of error messages
errorMessage: e.messageis rendered to users. Ensure allHttpErrormessages are pre-sanitized and contain no sensitive internals for routing forms.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/routing-forms/lib/getRoutedUrl.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/routing-forms/lib/getRoutedUrl.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/routing-forms/lib/getRoutedUrl.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/routing-forms/lib/getRoutedUrl.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
🧬 Code graph analysis (1)
packages/features/routing-forms/lib/getRoutedUrl.ts (1)
packages/platform/libraries/index.ts (1)
HttpError(59-59)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
| import { TRPCError } from "@trpc/server"; | ||
|
|
||
| import { getUrlSearchParamsToForward } from "./getUrlSearchParamsToForward"; | ||
| import { HttpError } from "@calcom/lib/http-error"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure instanceof works: unify HttpError source across throw/catch sites
If upstream throws a different HttpError constructor (e.g., via a platform re‑export) than the one imported here, instanceof HttpError won’t match and the catch path won’t run. Verify that all throwers and checkers import the same symbol from the same module.
Run to confirm a single canonical import path and where HttpError is thrown:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Unique import paths for HttpError:"
rg -nPo --type=ts 'import\s+{?\s*HttpError\s*}?\s+from\s+"[^"]+"' -g '!**/node_modules/**' \
| sed -E 's/.*from "([^"]+)".*/\1/' | sort -u
echo -e "\nSites throwing HttpError:"
rg -nP --type=ts '\bthrow\s+new\s+HttpError\s*\(' -g '!**/node_modules/**' -C2
echo -e "\nAll instanceof HttpError checks:"
rg -nP --type=ts 'instanceof\s+HttpError' -g '!**/node_modules/**' -C2Length of output: 183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Working dir: $(pwd)"
echo -e "\nQuick repo top-level listing:"
ls -la | sed -n '1,80p' || true
echo -e "\nUnique import paths for HttpError (searching all files, disabling ignore heuristics):"
rg -nP -uu -g '!**/node_modules/**' 'import\s+{?\s*HttpError\s*}?\s+from\s+["'\"'\'']?[^"'\''"]+["'\"'\'']' -o -S \
| sed -E 's/.*from ["'"'"']([^"'"'"']+)["'"'"'].*/\1/' | sort -u || true
echo -e "\nSites throwing HttpError:"
rg -nP -uu -g '!**/node_modules/**' '\bthrow\s+new\s+HttpError\s*\(' -n -S -C2 || true
echo -e "\nAll instanceof HttpError checks:"
rg -nP -uu -g '!**/node_modules/**' 'instanceof\s+HttpError' -n -S -C2 || true
echo -e "\nDefinitions / re-exports of HttpError (class, export, type, interface):"
rg -nP -uu -g '!**/node_modules/**' '(class\s+HttpError|export\s+.*HttpError|type\s+HttpError|interface\s+HttpError|export\s*\{\s*HttpError\s*\})' -n -S -C2 || trueLength of output: 4764
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Working dir: $(pwd)"
echo -e "\nAll occurrences of 'HttpError' (excluding node_modules):"
rg -n --hidden -uu -g '!**/node_modules/**' 'HttpError' -S || true
echo -e "\nImport statements referencing HttpError (showing surrounding 1 line):"
rg -n --hidden -uu -g '!**/node_modules/**' 'HttpError' -S -n -C1 | rg -n --hidden -v '^\s*$' || true
echo -e "\nSites throwing HttpError:"
rg -n --hidden -uu -g '!**/node_modules/**' 'throw\s+new\s+HttpError' -P -S || true
echo -e "\nInstanceof checks:"
rg -n --hidden -uu -g '!**/node_modules/**' 'instanceof\s+HttpError' -P -S || true
echo -e "\nDefinition / export sites (class/export/type/interface mentioning HttpError):"
rg -n --hidden -uu -g '!**/node_modules/**' -e 'class HttpError' -e 'interface HttpError' -e 'type HttpError' -e 'export.*HttpError' -S || trueLength of output: 1031
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Working dir: $(pwd)"
echo -e "\nShow packages/features/routing-forms/lib/getRoutedUrl.ts (if present):"
if [ -f "packages/features/routing-forms/lib/getRoutedUrl.ts" ]; then
sed -n '1,240p' packages/features/routing-forms/lib/getRoutedUrl.ts || true
else
echo "file not found: packages/features/routing-forms/lib/getRoutedUrl.ts"
fi
echo -e "\nSearching repository (excluding node_modules and .git) for 'HttpError' occurrences:"
find . -type f -not -path './node_modules/*' -not -path './.git/*' -print0 \
| xargs -0 grep -nI --color=never -e 'HttpError' || true
echo -e "\nUnique files containing HttpError:"
find . -type f -not -path './node_modules/*' -not -path './.git/*' -print0 \
| xargs -0 grep -lI -e 'HttpError' | sort -u || true
echo -e "\nImport lines referencing HttpError:"
find . -type f -not -path './node_modules/*' -not -path './.git/*' -print0 \
| xargs -0 grep -nI --color=never -E "import .*HttpError" || true
echo -e "\nOccurrences of 'throw new HttpError':"
find . -type f -not -path './node_modules/*' -not -path './.git/*' -print0 \
| xargs -0 grep -nI --color=never -E "throw\s+new\s+HttpError" || true
echo -e "\nOccurrences of 'instanceof HttpError':"
find . -type f -not -path './node_modules/*' -not -path './.git/*' -print0 \
| xargs -0 grep -nI --color=never -E "instanceof\s+HttpError" || true
echo -e "\nDefinitions or re-exports mentioning HttpError (class/type/interface/export):"
find . -type f -not -path './node_modules/*' -not -path './.git/*' -print0 \
| xargs -0 grep -nI --color=never -E "class\s+HttpError|interface\s+HttpError|type\s+HttpError|export\s*\{[^}]*HttpError|export\s+.*HttpError" || true
echo -e "\nSearch node_modules/@calcom (if present) for HttpError references:"
if [ -d "node_modules/@calcom" ]; then
grep -nI --color=never -R "HttpError" node_modules/@calcom || true
else
echo "node_modules/@calcom not present"
fiLength of output: 198468
Ensure instanceof works: unify HttpError source across throw/catch sites
Repository contains two distinct HttpError classes (packages/lib/http-error.ts and apps/web/lib/core/http/error/http-error.ts). packages/features/routing-forms/lib/getRoutedUrl.ts imports "@calcom/lib/http-error" (import at line 29) and uses if (e instanceof HttpError) (line 174) — that check will fail if a different HttpError constructor is thrown. Action: pick a single canonical HttpError (prefer @calcom/lib/http-error / packages/lib/http-error.ts), update all imports to that module (including app-local imports like @lib/core/http/error), or replace instanceof with a shared type guard (e.g., isHttpError(error)) and/or a stable property check.
🤖 Prompt for AI Agents
In packages/features/routing-forms/lib/getRoutedUrl.ts (import at line 29,
instanceof use at line 174): the repo has two distinct HttpError classes so `e
instanceof HttpError` can fail; pick a single canonical HttpError (prefer
packages/lib/http-error.ts exported as @calcom/lib/http-error) and update all
imports across the codebase (including app-local @lib/core/http/error) to import
that canonical module, OR implement and import a shared type guard (e.g.,
isHttpError(error) that checks a stable property like error.status or
error.code) and replace `instanceof HttpError` with that guard to ensure
consistent detection of HttpError instances.
| crmAppSlug = result.crmAppSlug; | ||
| } catch (e) { | ||
| if (e instanceof TRPCError) { | ||
| if (e instanceof HttpError) { |
There was a problem hiding this comment.
| if (e instanceof HttpError) { | |
| if (e instanceof HttpError || e instanceof TRPCError) { |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
E2E results are ready! |
5 participants
What does this PR do?
The change updates error handling in packages/features/routing-forms/lib/getRoutedUrl.ts by replacing TRPCError with HttpError. Two catch blocks now check for e instanceof HttpError and, when matched, return page props containing errorMessage set to e.message. Existing logging and rethrow behavior remain unchanged. Control flow is otherwise unaffected. There are no modifications to exported or public declarations. Minor whitespace adjustments were made.
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)