fix: validate BACKEND_URL before using it#40
Conversation
Skip backend calls entirely when BACKEND_URL is not configured or
contains an invalid value (like a literal ${...} template string).
This prevents URL parse errors on Vercel where the env var may not
be set.
https://claude.ai/code/session_015Pd3a6hinTenCNrPRGiZqE
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the application's robustness by introducing validation for the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
🚅 Deployed to the EventRelay-pr-40 environment in EventRelay
|
There was a problem hiding this comment.
Code Review
This pull request introduces a validation for the BACKEND_URL environment variable to prevent crashes when it's not set, which is a good improvement. The implementation correctly guards backend calls using a new BACKEND_AVAILABLE flag. However, I've identified a few areas for improvement. The URL validation logic is a bit weak and is duplicated across two files; I've suggested a more robust and DRY approach. Additionally, I found a regression in the health check endpoint (/api/video) that now reports an incorrect status if the backend is configured but unreachable, and I've provided a fix.
I am having trouble creating individual review comments. Click here to see my feedback.
apps/web/src/app/api/video/route.ts (172-204)
There's a logic regression in the health check endpoint. If BACKEND_AVAILABLE is true but the fetch call fails (e.g., the backend server is down), the empty catch block causes a fall-through to the "Frontend-only mode" response. This incorrectly reports backend_status: 'not-configured' when it should be 'unavailable' to indicate the backend is configured but unreachable.
export async function GET() {
// If backend URL is configured and valid, check its health
if (BACKEND_AVAILABLE) {
try {
const response = await fetch(`${BACKEND_URL}/api/v1/health`);
const health = await response.json();
return NextResponse.json({
name: 'UVAI Video Analysis API',
version: '2.0.0',
backend_status: health.status,
backend_components: health.components,
endpoints: {
analyze: 'POST /api/video - Analyze a video URL',
health: 'GET /api/video - Check API status',
},
});
} catch {
// Backend configured but unreachable
return NextResponse.json({
name: 'UVAI Video Analysis API',
version: '2.0.0',
backend_status: 'unavailable',
frontend_pipeline: 'active',
endpoints: {
analyze: 'POST /api/video - Analyze a video URL',
},
});
}
}
// Frontend-only mode
return NextResponse.json({
name: 'UVAI Video Analysis API',
version: '2.0.0',
backend_status: 'not-configured',
frontend_pipeline: 'active',
endpoints: {
analyze: 'POST /api/video - Analyze a video URL',
},
});
}apps/web/src/app/api/transcribe/route.ts (18-20)
The URL validation rawBackendUrl.startsWith('http') is not very robust. It would accept http as a valid start (which is not a valid protocol) and would reject https. Also, the check is performed twice, which is redundant.
Consider a more robust check and refactor to avoid repetition. This logic is also duplicated in apps/web/src/app/api/video/route.ts. It would be best to extract this to a shared utility file to avoid code duplication.
const rawBackendUrl = process.env.BACKEND_URL || '';
const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http://') || rawBackendUrl.startsWith('https://');
const BACKEND_URL = BACKEND_AVAILABLE ? rawBackendUrl : 'http://localhost:8000';
apps/web/src/app/api/video/route.ts (4-6)
The URL validation rawBackendUrl.startsWith('http') is not very robust. It would accept http as a valid start (which is not a valid protocol) and would reject https. Also, the check is performed twice, which is redundant.
Consider a more robust check and refactor to avoid repetition. This logic is also duplicated in apps/web/src/app/api/transcribe/route.ts. It would be best to extract this to a shared utility file to avoid code duplication.
const rawBackendUrl = process.env.BACKEND_URL || '';
const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http://') || rawBackendUrl.startsWith('https://');
const BACKEND_URL = BACKEND_AVAILABLE ? rawBackendUrl : 'http://localhost:8000';
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent serverless runtime failures by avoiding backend fetches when BACKEND_URL is unset or clearly invalid (e.g., literal ${...} placeholders), improving resilience on Vercel deployments.
Changes:
- Add
rawBackendUrlparsing and aBACKEND_AVAILABLEguard to conditionally call the backend. - Skip the backend pipeline in
/api/videoand backend transcript strategy in/api/transcribewhen the backend is not configured. - Update
/api/videoGETto only check backend health whenBACKEND_AVAILABLEis true.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/web/src/app/api/video/route.ts | Gate backend pipeline + health check behind BACKEND_AVAILABLE to avoid invalid backend URL usage. |
| apps/web/src/app/api/transcribe/route.ts | Gate backend YouTube transcript strategy behind BACKEND_AVAILABLE to avoid invalid backend URL usage. |
| const BACKEND_URL = rawBackendUrl.startsWith('http') ? rawBackendUrl : 'http://localhost:8000'; | ||
| const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http'); | ||
|
|
There was a problem hiding this comment.
rawBackendUrl.startsWith('http') is not actually validating that BACKEND_URL is a parseable http(s) URL (e.g., it would treat httpx://... or https://${...} as "available" and still hit fetch URL-parse errors). Consider validating with new URL(rawBackendUrl) (and checking protocol is http:/https:) and/or explicitly rejecting placeholder tokens like ${...} before setting BACKEND_AVAILABLE.
| const BACKEND_URL = rawBackendUrl.startsWith('http') ? rawBackendUrl : 'http://localhost:8000'; | |
| const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http'); | |
| let BACKEND_URL = 'http://localhost:8000'; | |
| let BACKEND_AVAILABLE = false; | |
| if (rawBackendUrl) { | |
| try { | |
| const parsed = new URL(rawBackendUrl); | |
| const isHttpProtocol = parsed.protocol === 'http:' || parsed.protocol === 'https:'; | |
| const hasPlaceholder = rawBackendUrl.includes('${'); | |
| if (isHttpProtocol && !hasPlaceholder) { | |
| BACKEND_URL = rawBackendUrl; | |
| BACKEND_AVAILABLE = true; | |
| } | |
| } catch { | |
| // Invalid URL in BACKEND_URL; fall back to defaults | |
| } | |
| } |
| }, | ||
| }); | ||
| } catch { | ||
| // Backend configured but unreachable |
There was a problem hiding this comment.
In GET, if BACKEND_AVAILABLE is true but the backend health check fails, the handler currently falls through to the frontend-only response with backend_status: 'not-configured', which misrepresents the state (it is configured but unreachable). Consider returning backend_status: 'unavailable' (or similar) when the fetch throws, and reserving 'not-configured' strictly for when BACKEND_AVAILABLE is false.
| // Backend configured but unreachable | |
| // Backend configured but unreachable | |
| return NextResponse.json({ | |
| name: 'UVAI Video Analysis API', | |
| version: '2.0.0', | |
| backend_status: 'unavailable', | |
| frontend_pipeline: 'active', | |
| endpoints: { | |
| analyze: 'POST /api/video - Analyze a video URL', | |
| health: 'GET /api/video - Check API status', | |
| }, | |
| }); |
| const BACKEND_URL = rawBackendUrl.startsWith('http') ? rawBackendUrl : 'http://localhost:8000'; | ||
| const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http'); | ||
|
|
There was a problem hiding this comment.
Same as in /api/video: rawBackendUrl.startsWith('http') is too permissive to be considered URL validation (e.g., httpx://... or https://${...} would be treated as available and can still trigger URL parse errors). Consider using new URL(rawBackendUrl) + protocol check (and optionally rejecting ${...} tokens) to decide BACKEND_AVAILABLE.
| const BACKEND_URL = rawBackendUrl.startsWith('http') ? rawBackendUrl : 'http://localhost:8000'; | |
| const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http'); | |
| let BACKEND_URL = 'http://localhost:8000'; | |
| let BACKEND_AVAILABLE = false; | |
| if (rawBackendUrl) { | |
| try { | |
| const parsed = new URL(rawBackendUrl); | |
| const protocolOk = parsed.protocol === 'http:' || parsed.protocol === 'https:'; | |
| const hasTemplateTokens = rawBackendUrl.includes('${'); | |
| if (protocolOk && !hasTemplateTokens) { | |
| BACKEND_URL = rawBackendUrl; | |
| BACKEND_AVAILABLE = true; | |
| } | |
| } catch { | |
| // Invalid BACKEND_URL; keep defaults | |
| } | |
| } |
| // Backend URL with validation - skip if not a valid URL | ||
| const rawBackendUrl = process.env.BACKEND_URL || ''; | ||
| const BACKEND_URL = rawBackendUrl.startsWith('http') ? rawBackendUrl : 'http://localhost:8000'; | ||
| const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http'); |
There was a problem hiding this comment.
Bug: The new BACKEND_AVAILABLE flag is false when process.env.BACKEND_URL is unset, which disables the documented localhost fallback and breaks local development workflows.
Severity: MEDIUM
Suggested Fix
The logic should differentiate between an unset environment variable and an invalid one. A potential fix is to set BACKEND_AVAILABLE to true if rawBackendUrl is empty, allowing the localhost fallback to be used. For example: const BACKEND_AVAILABLE = rawBackendUrl.startsWith('http') || rawBackendUrl === '';. This preserves the original fallback behavior for local development while still guarding against invalid template strings from deployment environments.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/web/src/app/api/transcribe/route.ts#L17-L20
Potential issue: The logic for setting `BACKEND_URL` was changed to handle cases where
the variable might be an invalid template string. However, this introduced a bug. When
`process.env.BACKEND_URL` is not set, `rawBackendUrl` becomes an empty string, causing
`BACKEND_AVAILABLE` to be `false`. Consequently, any code guarded by `if
(BACKEND_AVAILABLE)` will not execute, effectively disabling the documented localhost
fallback to `'http://localhost:8000'`. This breaks local development for users who rely
on the default behavior of running the backend on localhost without explicitly setting
the environment variable. This also creates an inconsistency, as some API routes were
not updated with this new logic.
Did we get this right? 👍 / 👎 to inform future reviews.
Skip backend calls entirely when BACKEND_URL is not configured or contains an invalid value (like a literal ${...} template string). This prevents URL parse errors on Vercel where the env var may not be set.
https://claude.ai/code/session_015Pd3a6hinTenCNrPRGiZqE