Skip to content

fix: catch HttpError in getRoutedUrl #23844

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.

Sign up for GitHub

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

Merged
ThyMinimalDev merged 4 commits into from
Sep 15, 2025
Merged

fix: catch HttpError in getRoutedUrl #23844

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2400435
fix: catch HttpError in getRoutedUrl
ThyMinimalDev Sep 15, 2025
a016898
fixup! fix: catch HttpError in getRoutedUrl
ThyMinimalDev Sep 15, 2025
578915f
Merge branch 'main' into catch-http-error-in-get-routed-url
ThyMinimalDev Sep 15, 2025
d353b48
Merge branch 'main' into catch-http-error-in-get-routed-url
ThyMinimalDev Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/features/routing-forms/lib/getRoutedUrl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import { withReporting } from "@calcom/lib/sentryWrapper";
import { PrismaRoutingFormRepository } from "@calcom/lib/server/repository/PrismaRoutingFormRepository";
import { UserRepository } from "@calcom/lib/server/repository/user";
import prisma from "@calcom/prisma";

import { TRPCError } from "@trpc/server";

import { getUrlSearchParamsToForward } from "./getUrlSearchParamsToForward";
import { HttpError } from "@calcom/lib/http-error";
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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/**' -C2

Length 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 || true

Length 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 || true

Length 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"
fi

Length 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.


const log = logger.getSubLogger({ prefix: ["[routing-forms]", "[router]"] });
const querySchema = z
Expand Down Expand Up @@ -171,7 +171,7 @@ const _getRoutedUrl = async (context: Pick<GetServerSidePropsContext, "query" |
crmContactOwnerRecordType = result.crmContactOwnerRecordType;
crmAppSlug = result.crmAppSlug;
} catch (e) {
if (e instanceof TRPCError) {
Copy link
Contributor Author

@ThyMinimalDev ThyMinimalDev Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hbjORbj linked to the changed made in #23768

Copy link
Contributor Author

@ThyMinimalDev ThyMinimalDev Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hariombalhara

Copy link
Member

@hariombalhara hariombalhara Sep 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be safer to keep both, considering that if some fn somewhere starts throwing TRPCError or maybe throwing it currently.

Copy link
Contributor Author

@ThyMinimalDev ThyMinimalDev Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

if (e instanceof HttpError || e instanceof TRPCError) {
return {
props: {
...pageProps,
Expand All @@ -181,7 +181,7 @@ const _getRoutedUrl = async (context: Pick<GetServerSidePropsContext, "query" |
},
};
}

log.error("Error handling the response", safeStringify(e));
throw new Error("Error handling the response");
}
Expand Down
Loading