fix: improve API key validation and error handling in import modals #2453
Conversation
- Add server-side API key validation for Rebrandly and Short.io - Implement proper input patterns and length validation - Fix duplicate toast notifications on validation errors - Prevent continuous polling after errors to reduce layout shifts - Show specific error messages from backend validation
|
@BharathxD is attempting to deploy a commit to the Dub Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughValidation of API keys for Rebrandly and Short.io is now performed before saving them. This is achieved by sending a verification request to the respective external APIs. UI modals for importing these keys have improved error handling, input validation, and more informative error messages, enhancing the user experience during API key submission. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal (UI)
participant API Route
participant External API
User->>Modal (UI): Submit API key
Modal (UI)->>API Route: PUT request with API key
API Route->>External API: HEAD request to validate key
External API-->>API Route: Response (OK or error)
alt Key valid
API Route->>API Route: Save key in Redis
API Route-->>Modal (UI): Success response
Modal (UI)-->>User: Show success
else Key invalid
API Route-->>Modal (UI): Error with specific message
Modal (UI)-->>User: Show error message
end
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/api/workspaces/[idOrSlug]/import/rebrandly/route.ts(1 hunks)apps/web/app/api/workspaces/[idOrSlug]/import/short/route.ts(1 hunks)apps/web/ui/modals/import-rebrandly-modal.tsx(6 hunks)apps/web/ui/modals/import-short-modal.tsx(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (8)
apps/web/ui/modals/import-rebrandly-modal.tsx (4)
41-41: LGTM: Error state tracking improves UX.Good addition of error state tracking to prevent continuous loading indicators after errors.
95-96: LGTM: Improved loading logic prevents UI flicker.The conditional loading logic effectively prevents repeated loading states after errors, improving the user experience.
239-241: LGTM: Enhanced error message extraction.Good improvement to extract specific error messages from the API response instead of showing generic error messages.
268-271: LGTM: Proper input validation constraints.The validation constraints (32-character hexadecimal pattern) align perfectly with Rebrandly's API key format and the backend validation logic.
apps/web/ui/modals/import-short-modal.tsx (4)
42-42: LGTM: Consistent error state tracking.The error state tracking implementation is consistent with the Rebrandly modal and improves the user experience.
90-91: LGTM: Improved loading state management.The loading logic correctly prevents repeated loading indicators after errors, consistent with the Rebrandly implementation.
249-250: LGTM: Enhanced error messaging.Good improvement to display specific error messages from the backend validation instead of generic error toasts.
278-281: LGTM: Accurate input validation for Short.io format.The validation constraints correctly enforce the Short.io API key format (
sk_followed by 16 alphanumeric characters) and align with the backend validation.
| 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", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ 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.
| 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 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", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ 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.
| 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.
steven-tey
force-pushed
the
main
branch
2 times, most recently
from
ea17ddf to
f7d87c3
Compare
Problem
The import modals for Rebrandly and Short.io had several UX issues:
fix-import-modal-api-validation.mov
Solution
This PR addresses these issues with comprehensive improvements:
Backend Changes
Frontend Changes
Validation Patterns
[0-9a-f]{32}sk_[A-Za-z0-9]{16}Testing
Type of Change
Summary by CodeRabbit