Skip to content

fix: improve API key validation and error handling in import modals #2453

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+61 −5
Open

fix: improve API key validation and error handling in import modals #2453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4fe9fd0
fix: improve API key validation and error handling in import modals
BharathxD May 29, 2025
70d20e6
Merge branch 'dubinc:main' into fix/import-modal-api-validation
BharathxD May 29, 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
16 changes: 16 additions & 0 deletions apps/web/app/api/workspaces/[idOrSlug]/import/rebrandly/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,22 @@ export const GET = withWorkspace(async ({ workspace }) => {
// PUT /api/workspaces/[idOrSlug]/import/rebrandly - save Rebrandly API key
export const PUT = withWorkspace(async ({ req, workspace }) => {
const { apiKey } = await req.json();
const isValidApiKeyResponse = await fetch(
"https://api.rebrandly.com/v1/account",
{
method: "HEAD",
headers: {
"Content-Type": "application/json",
apikey: apiKey,
},
},
);
if (!isValidApiKeyResponse.ok) {
throw new DubApiError({
code: "bad_request",
message: "Invalid Rebrandly API key",
});
}
Comment on lines +82 to +97
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for network failures and timeouts.

The API key validation logic is correct, but it lacks proper error handling for network failures, timeouts, or API service unavailability.

Consider wrapping the fetch in a try-catch block and adding a timeout:

-  const isValidApiKeyResponse = await fetch(
-    "https://api.rebrandly.com/v1/account",
-    {
-      method: "HEAD",
-      headers: {
-        "Content-Type": "application/json",
-        apikey: apiKey,
-      },
-    },
-  );
-  if (!isValidApiKeyResponse.ok) {
-    throw new DubApiError({
-      code: "bad_request",
-      message: "Invalid Rebrandly API key",
-    });
-  }
+  try {
+    const isValidApiKeyResponse = await fetch(
+      "https://api.rebrandly.com/v1/account",
+      {
+        method: "HEAD",
+        headers: {
+          "Content-Type": "application/json",
+          apikey: apiKey,
+        },
+        signal: AbortSignal.timeout(10000), // 10 second timeout
+      },
+    );
+    if (!isValidApiKeyResponse.ok) {
+      throw new DubApiError({
+        code: "bad_request",
+        message: "Invalid Rebrandly API key",
+      });
+    }
+  } catch (error) {
+    if (error instanceof DubApiError) {
+      throw error;
+    }
+    throw new DubApiError({
+      code: "bad_request",
+      message: "Unable to validate Rebrandly API key. Please try again.",
+    });
+  }
📝 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.

Suggested change
const isValidApiKeyResponse = await fetch(
"https://api.rebrandly.com/v1/account",
{
method: "HEAD",
headers: {
"Content-Type": "application/json",
apikey: apiKey,
},
},
);
if (!isValidApiKeyResponse.ok) {
throw new DubApiError({
code: "bad_request",
message: "Invalid Rebrandly API key",
});
}
try {
const isValidApiKeyResponse = await fetch(
"https://api.rebrandly.com/v1/account",
{
method: "HEAD",
headers: {
"Content-Type": "application/json",
apikey: apiKey,
},
signal: AbortSignal.timeout(10000), // 10 second timeout
},
);
if (!isValidApiKeyResponse.ok) {
throw new DubApiError({
code: "bad_request",
message: "Invalid Rebrandly API key",
});
}
} catch (error) {
if (error instanceof DubApiError) {
throw error;
}
throw new DubApiError({
code: "bad_request",
message: "Unable to validate Rebrandly API key. Please try again.",
});
}
🤖 Prompt for AI Agents 
In apps/web/app/api/workspaces/[idOrSlug]/import/rebrandly/route.ts around lines
82 to 97, the fetch call validating the Rebrandly API key lacks error handling
for network failures, timeouts, or service unavailability. Wrap the fetch call
in a try-catch block to catch exceptions from network errors, and implement a
timeout mechanism to abort the request if it takes too long. In the catch block,
throw a DubApiError with an appropriate message indicating the network or
timeout issue.

const response = await redis.set(`import:rebrandly:${workspace.id}`, apiKey);
return NextResponse.json(response);
});
Expand Down
16 changes: 16 additions & 0 deletions apps/web/app/api/workspaces/[idOrSlug]/import/short/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,22 @@ export const GET = withWorkspace(async ({ workspace }) => {
// PUT /api/workspaces/[idOrSlug]/import/short - save Short.io API key
export const PUT = withWorkspace(async ({ req, workspace }) => {
const { apiKey } = await req.json();
const isValidApiKeyResponse = await fetch(
"https://api.short.io/api/domains?limit=1&offset=0",
{
method: "HEAD",
headers: {
"Content-Type": "application/json",
Authorization: apiKey,
},
},
);
if (!isValidApiKeyResponse.ok) {
throw new DubApiError({
code: "bad_request",
message: "Invalid Short.io API key",
});
}
Comment on lines +69 to +84
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add consistent error handling for network failures.

The validation logic mirrors the Rebrandly implementation but has the same network error handling limitations. Consider applying the same timeout and error handling improvements suggested for the Rebrandly route.

Apply similar error handling as recommended for Rebrandly:

-  const isValidApiKeyResponse = await fetch(
-    "https://api.short.io/api/domains?limit=1&offset=0",
-    {
-      method: "HEAD",
-      headers: {
-        "Content-Type": "application/json",
-        Authorization: apiKey,
-      },
-    },
-  );
-  if (!isValidApiKeyResponse.ok) {
-    throw new DubApiError({
-      code: "bad_request",
-      message: "Invalid Short.io API key",
-    });
-  }
+  try {
+    const isValidApiKeyResponse = await fetch(
+      "https://api.short.io/api/domains?limit=1&offset=0",
+      {
+        method: "HEAD",
+        headers: {
+          "Content-Type": "application/json",
+          Authorization: apiKey,
+        },
+        signal: AbortSignal.timeout(10000), // 10 second timeout
+      },
+    );
+    if (!isValidApiKeyResponse.ok) {
+      throw new DubApiError({
+        code: "bad_request",
+        message: "Invalid Short.io API key",
+      });
+    }
+  } catch (error) {
+    if (error instanceof DubApiError) {
+      throw error;
+    }
+    throw new DubApiError({
+      code: "bad_request",
+      message: "Unable to validate Short.io API key. Please try again.",
+    });
+  }
📝 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.

Suggested change
const isValidApiKeyResponse = await fetch(
"https://api.short.io/api/domains?limit=1&offset=0",
{
method: "HEAD",
headers: {
"Content-Type": "application/json",
Authorization: apiKey,
},
},
);
if (!isValidApiKeyResponse.ok) {
throw new DubApiError({
code: "bad_request",
message: "Invalid Short.io API key",
});
}
try {
const isValidApiKeyResponse = await fetch(
"https://api.short.io/api/domains?limit=1&offset=0",
{
method: "HEAD",
headers: {
"Content-Type": "application/json",
Authorization: apiKey,
},
signal: AbortSignal.timeout(10000), // 10 second timeout
},
);
if (!isValidApiKeyResponse.ok) {
throw new DubApiError({
code: "bad_request",
message: "Invalid Short.io API key",
});
}
} catch (error) {
if (error instanceof DubApiError) {
throw error;
}
throw new DubApiError({
code: "bad_request",
message: "Unable to validate Short.io API key. Please try again.",
});
}
🤖 Prompt for AI Agents 
In apps/web/app/api/workspaces/[idOrSlug]/import/short/route.ts around lines 69
to 84, the fetch call validating the Short.io API key lacks consistent error
handling for network failures and timeouts. To fix this, wrap the fetch call in
a try-catch block to handle exceptions, implement a timeout mechanism to abort
the request if it takes too long, and throw a meaningful error if the network
request fails or times out, similar to the approach used in the Rebrandly route.

const response = await redis.set(`import:short:${workspace.id}`, apiKey);
return NextResponse.json(response);
});
Expand Down
16 changes: 14 additions & 2 deletions apps/web/ui/modals/import-rebrandly-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ function ImportRebrandlyModal({

const folderId = searchParams.get("folderId");

const [hasEncounteredError, setHasEncounteredError] = useState(false);

const {
data: { domains, tagsCount } = {
domains: null,
Expand All @@ -55,10 +57,12 @@ function ImportRebrandlyModal({
fetcher,
{
onError: (err) => {
setHasEncounteredError(true);
if (err.message !== "No Rebrandly access token found") {
toast.error(err.message);
}
},
onSuccess: () => setHasEncounteredError(false),
},
);

Expand Down Expand Up @@ -88,6 +92,9 @@ function ImportRebrandlyModal({

const { isMobile } = useMediaQuery();

// Only show loading if we haven't encountered an error before and workspaceId exists
const shouldShowLoading = (isLoading && !hasEncounteredError) || !workspaceId;

return (
<Modal
showModal={showImportRebrandlyModal}
Expand Down Expand Up @@ -116,7 +123,7 @@ function ImportRebrandlyModal({
</div>

<div className="flex flex-col space-y-6 bg-neutral-50 px-4 py-8 text-left sm:px-16">
{isLoading || !workspaceId ? (
{shouldShowLoading ? (
<div className="flex flex-col items-center justify-center space-y-4 bg-none">
<LoadingSpinner />
<p className="text-sm text-neutral-500">Connecting to Rebrandly</p>
Expand Down Expand Up @@ -229,7 +236,8 @@ function ImportRebrandlyModal({
await mutate();
toast.success("Successfully added API key");
} else {
toast.error("Error adding API key");
const body = await res.json();
toast.error(body.error?.message || "Error adding API key");
}
setSubmitting(false);
});
Expand Down Expand Up @@ -257,6 +265,10 @@ function ImportRebrandlyModal({
autoFocus={!isMobile}
type="text"
placeholder="93467061146a64622df83c12bcc0bffb"
minLength={32}
maxLength={32}
pattern="[0-9a-f]{32}"
title="Please enter a valid Rebrandly API key"
autoComplete="off"
required
className="mt-1 block w-full appearance-none rounded-md border border-neutral-300 px-3 py-2 placeholder-neutral-400 shadow-sm focus:border-black focus:outline-none focus:ring-black sm:text-sm"
Expand Down
18 changes: 15 additions & 3 deletions apps/web/ui/modals/import-short-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ function ImportShortModal({

const folderId = searchParams.get("folderId");

const [hasEncounteredError, setHasEncounteredError] = useState(false);

const {
data: domains,
isLoading,
Expand All @@ -50,10 +52,12 @@ function ImportShortModal({
fetcher,
{
onError: (err) => {
setHasEncounteredError(true);
if (err.message !== "No Short.io access token found") {
toast.error(err.message);
}
},
onSuccess: () => setHasEncounteredError(false),
},
);

Expand Down Expand Up @@ -83,6 +87,9 @@ function ImportShortModal({

const { isMobile } = useMediaQuery();

// Only show loading if we haven't encountered an error before and workspaceId exists
const shouldShowLoading = (isLoading && !hasEncounteredError) || !workspaceId;

return (
<Modal
showModal={showImportShortModal}
Expand Down Expand Up @@ -111,7 +118,7 @@ function ImportShortModal({
</div>

<div className="flex flex-col space-y-6 bg-neutral-50 px-4 py-8 text-left sm:px-16">
{isLoading || !workspaceId ? (
{shouldShowLoading ? (
<div className="flex flex-col items-center justify-center space-y-4 bg-none">
<LoadingSpinner />
<p className="text-sm text-neutral-500">Connecting to Short.io</p>
Expand Down Expand Up @@ -147,7 +154,7 @@ function ImportShortModal({
{
loading: "Adding links to import queue...",
success:
"Successfully added links to import queue! You can now safely navigate from this tab – we will send you an email when your links have been fully imported.",
"Successfully added links to import queue! You can now safely navigate from this tab – we will send you an email when your links have been fully imported.",
error: "Error adding links to import queue",
},
);
Expand Down Expand Up @@ -239,7 +246,8 @@ function ImportShortModal({
await mutate();
toast.success("Successfully added API key");
} else {
toast.error("Error adding API key");
const body = await res.json();
toast.error(body.error?.message || "Error adding API key");
}
setSubmitting(false);
});
Expand Down Expand Up @@ -267,6 +275,10 @@ function ImportShortModal({
autoFocus={!isMobile}
type="text"
placeholder="sk_xxxxxxxxxxxxxxxx"
minLength={19}
maxLength={19}
pattern="sk_[A-Za-z0-9]{16}"
title="Please enter a valid Short.io API key"
autoComplete="off"
required
className="mt-1 block w-full appearance-none rounded-md border border-neutral-300 px-3 py-2 placeholder-neutral-400 shadow-sm focus:border-black focus:outline-none focus:ring-black sm:text-sm"
Expand Down