Conversation
📝 WalkthroughWalkthroughSix new Next.js API route handlers are added to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@app/api/evaluations/stt/datasets/`[dataset_id]/route.ts:
- Around line 20-24: The catch block currently returns an inaccurate error
message ("Failed to fetch config"); update the response in the catch inside
route.ts so the NextResponse.json call uses a message referencing the dataset
(e.g., "Failed to fetch dataset" or include the dataset_id) instead of "config".
Locate the catch block that calls NextResponse.json and replace the error string
to clearly reference the dataset (and optionally include dataset_id or error
details if available) so the returned error message matches the operation
performed.
In `@app/api/evaluations/stt/datasets/route.ts`:
- Around line 56-61: The catch block in the POST handler currently assumes error
has a .message property (console.error and NextResponse.json using
error.message); update the POST catch to normalize the error into a safe string
(e.g., const msg = error instanceof Error ? error.message : String(error)) and
use that safe value in both the console.error and the NextResponse.json details
so we don't access .message on unknown types; look for the catch in the POST
route handler in route.ts and replace direct error.message usage with this
normalized message before returning the response.
- Around line 18-22: The catch block in the route handler currently assumes
`error.message` exists; update the error handling in the catch clause to narrow
the unknown `error` with `if (error instanceof Error)` and use `error.message`
when true, otherwise provide a safe fallback string (e.g., "Unknown error") when
building the NextResponse JSON; locate and update the catch in the evaluation
STT datasets route (the block that returns NextResponse.json({ success: false,
error: error.message, ... })) to implement this instanceof Error narrowing and
fallback.
- Around line 16-17: The handler currently returns NextResponse.json(data) but
does not forward the upstream HTTP status (same bug as runs/route.ts); change
the return to preserve the original response status (and optionally headers) by
returning NextResponse.json(data, { status: response.status /*, headers:
response.headers */ }); locate the variables response and data in route.ts and
update the NextResponse.json call accordingly.
In `@app/api/evaluations/stt/results/`[result_id]/route.ts:
- Line 8: The code uses a NEXT_PUBLIC_ prefixed env var for server-only route
handlers, which leaks to the client; update the backendUrl constant to read a
non-public variable (e.g., process.env.BACKEND_URL) with the existing fallback
('http://localhost:8000') and replace occurrences in this file's backendUrl
declaration (and the same change in the other five route files in this PR) so
the backend URL is read from a server-only env var instead of
NEXT_PUBLIC_BACKEND_URL.
- Around line 20-24: The catch block returns an inaccurate error message
("Failed to fetch config") — update the NextResponse.json call in the route's
catch handler to return a correct message like "Failed to fetch result" (keep
success:false, data:null and status:500) so the error reflects the endpoint's
purpose; apply the same fix to the analogous catch handlers in the
datasets/[dataset_id] and runs/[run_id] routes, locating the places by searching
for NextResponse.json(...) in the catch blocks of those route handlers.
In `@app/api/evaluations/stt/runs/`[run_id]/route.ts:
- Around line 20-24: The catch block in route.ts returns an inaccurate error
message; update the NextResponse.json call inside the catch (where it currently
returns { success: false, error: 'Failed to fetch config', ... }) to reference
the run (e.g., 'Failed to fetch run' or include the run_id) so the error
correctly describes the failed operation; modify the error string passed to
NextResponse.json in that catch block (the code around NextResponse.json)
accordingly.
In `@app/api/evaluations/stt/runs/route.ts`:
- Around line 18-22: The catch blocks currently access error.message on an
unknown-typed catch variable (seen where NextResponse.json is called in the
catch), which breaks TypeScript strict checks; update each catch (including in
app/api/evaluations/stt/runs/route.ts, datasets/route.ts and runs/route.ts) to
safely narrow the error before reading message—e.g. determine const msg = error
instanceof Error ? error.message : String(error) (or JSON.stringify when
appropriate) and pass msg into NextResponse.json instead of error.message so the
code compiles under strict/useUnknownInCatchVariables.
- Around line 56-61: The catch block in the POST handler currently uses
error.message which will throw if error isn't an Error; update the POST catch
block (the try/catch around the request forward) to safely extract a message by
checking if error is an instance of Error and using error.message, otherwise
convert the error to a string (e.g., String(error) or JSON.stringify fallback)
and include that safe message in the NextResponse.json details and the
console.error call so unknown error shapes don't cause a secondary failure.
- Around line 16-17: The GET handler currently returns the backend JSON body but
drops the original HTTP status; update the response to forward the backend
status by returning NextResponse.json(data, { status: response.status }) instead
of NextResponse.json(data). Apply the same change to the analogous GET handler
in datasets/route.ts so both app/api/evaluations/stt/runs/route.ts and
datasets/route.ts forward response.status via NextResponse.json.
🧹 Nitpick comments (6)
app/api/evaluations/stt/results/[result_id]/route.ts (1)
1-1:NextRequestis imported but unused.The
requestparameter is typed asRequest, notNextRequest. Either useNextRequestas the parameter type or remove the unused import.Proposed fix
-import { NextResponse, NextRequest } from 'next/server'; +import { NextResponse } from 'next/server';app/api/evaluations/stt/datasets/[dataset_id]/route.ts (1)
1-1: UnusedNextRequestimport — same as the other route files.app/api/evaluations/stt/runs/[run_id]/route.ts (1)
1-1: UnusedNextRequestimport.app/api/evaluations/stt/files/route.ts (3)
37-41: Redundant branch — both paths return the identical response.The
if (!response.ok)check returns the exact same expression as the else branch. Remove the conditional.Proposed fix
- // Return the response with the same status code - if (!response.ok) { - return NextResponse.json(data, { status: response.status }); - } - - return NextResponse.json(data, { status: response.status }); + return NextResponse.json(data, { status: response.status });
42-42: Avoid explicitany— use proper type narrowing instead.Typing
errorasanysuppresses useful type safety. Prefer theunknown+instanceofpattern used elsewhere or at minimum keep itunknown.Proposed fix
- } catch (error: any) { + } catch (error) { console.error('Proxy error:', error); return NextResponse.json( - { error: 'Failed to forward request to backend', details: error.message }, + { error: 'Failed to forward request to backend', details: error instanceof Error ? error.message : 'Unknown error' }, { status: 500 } ); }
1-49: Consider extracting a shared proxy utility to reduce duplication across all six route files.All six route handlers repeat the same pattern: read env var, forward API key, fetch backend, return JSON, catch errors. A small shared helper (e.g.,
lib/proxy.ts) accepting the path and options would eliminate ~80% of the boilerplate and make error handling/status forwarding consistent in one place.
| } catch (error) { | ||
| return NextResponse.json( | ||
| { success: false, error: 'Failed to fetch config', data: null }, | ||
| { status: 500 } | ||
| ); |
There was a problem hiding this comment.
Error message is inaccurate — should reference "dataset", not "config".
Proposed fix
- { success: false, error: 'Failed to fetch config', data: null },
+ { success: false, error: 'Failed to fetch dataset', data: null },📝 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.
| } catch (error) { | |
| return NextResponse.json( | |
| { success: false, error: 'Failed to fetch config', data: null }, | |
| { status: 500 } | |
| ); | |
| } catch (error) { | |
| return NextResponse.json( | |
| { success: false, error: 'Failed to fetch dataset', data: null }, | |
| { status: 500 } | |
| ); |
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/datasets/`[dataset_id]/route.ts around lines 20 - 24,
The catch block currently returns an inaccurate error message ("Failed to fetch
config"); update the response in the catch inside route.ts so the
NextResponse.json call uses a message referencing the dataset (e.g., "Failed to
fetch dataset" or include the dataset_id) instead of "config". Locate the catch
block that calls NextResponse.json and replace the error string to clearly
reference the dataset (and optionally include dataset_id or error details if
available) so the returned error message matches the operation performed.
| const data = await response.json(); | ||
| return NextResponse.json(data); |
There was a problem hiding this comment.
Backend status not forwarded — same issue as runs/route.ts.
Proposed fix
const data = await response.json();
- return NextResponse.json(data);
+ return NextResponse.json(data, { status: response.status });📝 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 data = await response.json(); | |
| return NextResponse.json(data); | |
| const data = await response.json(); | |
| return NextResponse.json(data, { status: response.status }); |
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/datasets/route.ts` around lines 16 - 17, The handler
currently returns NextResponse.json(data) but does not forward the upstream HTTP
status (same bug as runs/route.ts); change the return to preserve the original
response status (and optionally headers) by returning NextResponse.json(data, {
status: response.status /*, headers: response.headers */ }); locate the
variables response and data in route.ts and update the NextResponse.json call
accordingly.
| } catch (error) { | ||
| return NextResponse.json( | ||
| { success: false, error: error.message, data: null }, | ||
| { status: 500 } | ||
| ); |
There was a problem hiding this comment.
error.message on unknown — same TypeScript issue as runs/route.ts.
Apply the same instanceof Error narrowing here.
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/datasets/route.ts` around lines 18 - 22, The catch
block in the route handler currently assumes `error.message` exists; update the
error handling in the catch clause to narrow the unknown `error` with `if (error
instanceof Error)` and use `error.message` when true, otherwise provide a safe
fallback string (e.g., "Unknown error") when building the NextResponse JSON;
locate and update the catch in the evaluation STT datasets route (the block that
returns NextResponse.json({ success: false, error: error.message, ... })) to
implement this instanceof Error narrowing and fallback.
| } catch (error) { | ||
| console.error('Proxy error:', error); | ||
| return NextResponse.json( | ||
| {error:'Failed to forward request', details:error.message}, | ||
| {status:500} | ||
| ); |
There was a problem hiding this comment.
error.message on unknown in POST catch block — same fix needed.
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/datasets/route.ts` around lines 56 - 61, The catch
block in the POST handler currently assumes error has a .message property
(console.error and NextResponse.json using error.message); update the POST catch
to normalize the error into a safe string (e.g., const msg = error instanceof
Error ? error.message : String(error)) and use that safe value in both the
console.error and the NextResponse.json details so we don't access .message on
unknown types; look for the catch in the POST route handler in route.ts and
replace direct error.message usage with this normalized message before returning
the response.
| { params }: { params: Promise<{ result_id: string }> } | ||
| ) { | ||
| const { result_id } = await params; | ||
| const backendUrl = process.env.NEXT_PUBLIC_BACKEND_URL || 'http://localhost:8000'; |
There was a problem hiding this comment.
NEXT_PUBLIC_ prefix exposes the backend URL to the client bundle.
Environment variables prefixed with NEXT_PUBLIC_ are inlined into the client-side JavaScript bundle. Since this is a server-only route handler, use a non-prefixed variable (e.g., BACKEND_URL) to avoid leaking internal infrastructure URLs to the browser. This applies to all six route files in this PR.
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/results/`[result_id]/route.ts at line 8, The code
uses a NEXT_PUBLIC_ prefixed env var for server-only route handlers, which leaks
to the client; update the backendUrl constant to read a non-public variable
(e.g., process.env.BACKEND_URL) with the existing fallback
('http://localhost:8000') and replace occurrences in this file's backendUrl
declaration (and the same change in the other five route files in this PR) so
the backend URL is read from a server-only env var instead of
NEXT_PUBLIC_BACKEND_URL.
| } catch (error) { | ||
| return NextResponse.json( | ||
| { success: false, error: 'Failed to fetch config', data: null }, | ||
| { status: 500 } | ||
| ); |
There was a problem hiding this comment.
Error message is inaccurate — copy-paste artifact.
The error says 'Failed to fetch config' but this endpoint fetches a result, not a config. The same copy-paste error appears in the datasets/[dataset_id] and runs/[run_id] routes as well.
Proposed fix
- { success: false, error: 'Failed to fetch config', data: null },
+ { success: false, error: 'Failed to fetch result', data: null },📝 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.
| } catch (error) { | |
| return NextResponse.json( | |
| { success: false, error: 'Failed to fetch config', data: null }, | |
| { status: 500 } | |
| ); | |
| } catch (error) { | |
| return NextResponse.json( | |
| { success: false, error: 'Failed to fetch result', data: null }, | |
| { status: 500 } | |
| ); |
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/results/`[result_id]/route.ts around lines 20 - 24,
The catch block returns an inaccurate error message ("Failed to fetch config") —
update the NextResponse.json call in the route's catch handler to return a
correct message like "Failed to fetch result" (keep success:false, data:null and
status:500) so the error reflects the endpoint's purpose; apply the same fix to
the analogous catch handlers in the datasets/[dataset_id] and runs/[run_id]
routes, locating the places by searching for NextResponse.json(...) in the catch
blocks of those route handlers.
| } catch (error) { | ||
| return NextResponse.json( | ||
| { success: false, error: 'Failed to fetch config', data: null }, | ||
| { status: 500 } | ||
| ); |
There was a problem hiding this comment.
Error message is inaccurate — should reference "run", not "config".
Proposed fix
- { success: false, error: 'Failed to fetch config', data: null },
+ { success: false, error: 'Failed to fetch run', data: null },🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/runs/`[run_id]/route.ts around lines 20 - 24, The
catch block in route.ts returns an inaccurate error message; update the
NextResponse.json call inside the catch (where it currently returns { success:
false, error: 'Failed to fetch config', ... }) to reference the run (e.g.,
'Failed to fetch run' or include the run_id) so the error correctly describes
the failed operation; modify the error string passed to NextResponse.json in
that catch block (the code around NextResponse.json) accordingly.
| const data = await response.json(); | ||
| return NextResponse.json(data); |
There was a problem hiding this comment.
Backend HTTP status is silently dropped — client always receives 200.
Unlike the [run_id] sub-route (which passes { status: response.status }), this GET handler returns the data without forwarding the backend's status code. A 404 or 403 from the backend will appear as a 200 to the client. The same issue exists in datasets/route.ts GET (Line 17).
Proposed fix
const data = await response.json();
- return NextResponse.json(data);
+ return NextResponse.json(data, { status: response.status });📝 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 data = await response.json(); | |
| return NextResponse.json(data); | |
| const data = await response.json(); | |
| return NextResponse.json(data, { status: response.status }); |
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/runs/route.ts` around lines 16 - 17, The GET handler
currently returns the backend JSON body but drops the original HTTP status;
update the response to forward the backend status by returning
NextResponse.json(data, { status: response.status }) instead of
NextResponse.json(data). Apply the same change to the analogous GET handler in
datasets/route.ts so both app/api/evaluations/stt/runs/route.ts and
datasets/route.ts forward response.status via NextResponse.json.
| } catch (error) { | ||
| return NextResponse.json( | ||
| { success: false, error: error.message, data: null }, | ||
| { status: 500 } | ||
| ); |
There was a problem hiding this comment.
error.message on unknown type will fail TypeScript strict checks.
In TypeScript, the caught error is typed as unknown by default. Accessing .message directly without narrowing will cause a compile error under strict / useUnknownInCatchVariables. This same issue appears in datasets/route.ts Lines 20 and 59, and runs/route.ts Line 59.
Proposed fix
} catch (error) {
return NextResponse.json(
- { success: false, error: error.message, data: null },
+ { success: false, error: error instanceof Error ? error.message : 'Internal server error', data: null },
{ status: 500 }
);
}📝 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.
| } catch (error) { | |
| return NextResponse.json( | |
| { success: false, error: error.message, data: null }, | |
| { status: 500 } | |
| ); | |
| } catch (error) { | |
| return NextResponse.json( | |
| { success: false, error: error instanceof Error ? error.message : 'Internal server error', data: null }, | |
| { status: 500 } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/runs/route.ts` around lines 18 - 22, The catch blocks
currently access error.message on an unknown-typed catch variable (seen where
NextResponse.json is called in the catch), which breaks TypeScript strict
checks; update each catch (including in app/api/evaluations/stt/runs/route.ts,
datasets/route.ts and runs/route.ts) to safely narrow the error before reading
message—e.g. determine const msg = error instanceof Error ? error.message :
String(error) (or JSON.stringify when appropriate) and pass msg into
NextResponse.json instead of error.message so the code compiles under
strict/useUnknownInCatchVariables.
| } catch (error) { | ||
| console.error('Proxy error:', error); | ||
| return NextResponse.json( | ||
| {error:'Failed to forward request', details:error.message}, | ||
| {status:500} | ||
| ); |
There was a problem hiding this comment.
Same error.message on unknown issue in the POST catch block.
Proposed fix
} catch (error) {
console.error('Proxy error:', error);
return NextResponse.json(
- {error:'Failed to forward request', details:error.message},
+ {error:'Failed to forward request', details: error instanceof Error ? error.message : 'Unknown error'},
{status:500}
);
}🤖 Prompt for AI Agents
In `@app/api/evaluations/stt/runs/route.ts` around lines 56 - 61, The catch block
in the POST handler currently uses error.message which will throw if error isn't
an Error; update the POST catch block (the try/catch around the request forward)
to safely extract a message by checking if error is an instance of Error and
using error.message, otherwise convert the error to a string (e.g.,
String(error) or JSON.stringify fallback) and include that safe message in the
NextResponse.json details and the console.error call so unknown error shapes
don't cause a secondary failure.
1 participant
Targer issue is #42
Summary by CodeRabbit