Skip to content

Add small condition to prevent unhandled error for search anywhere#8609

Merged
gmazoyer merged 2 commits intodevelopfrom
gma-20260316-fix-search-without-docs
Mar 18, 2026
Merged

Add small condition to prevent unhandled error for search anywhere#8609
gmazoyer merged 2 commits intodevelopfrom
gma-20260316-fix-search-without-docs

Conversation

@gmazoyer
Copy link
Contributor

@gmazoyer gmazoyer commented Mar 16, 2026

When trying to use the search anywhere feature, if the docs is not available, the frontend code would fail and display an unexpected error page.

Summary by CodeRabbit

  • Bug Fixes
    • API requests now check HTTP response status and report errors with status information instead of treating failed responses as successful.
    • Results in clearer, more reliable error messages for network/API failures, improving troubleshooting and overall stability.

When trying to use the search anywhere feature, if the docs is not
available, the frontend code would fail and display an unexpected error
page.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb848187-2754-41fe-bf72-8f72218fad0c

📥 Commits

Reviewing files that changed from the base of the PR and between 6a1cd81 and 19e8fb6.

📒 Files selected for processing (1)
  • frontend/app/src/shared/api/rest/fetch.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/app/src/shared/api/rest/fetch.ts

Walkthrough

A fetch utility in the API layer was modified to validate HTTP responses before parsing. The function now checks rawResponse.ok; if the response is not ok it throws an Error containing the HTTP status, and only when the response is successful does it call rawResponse.json() and return the parsed payload.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description identifies the problem and solution but lacks detail on implementation, testing approach, and impact assessment as suggested by the template. Expand the description to include: what the conditional check does, how to test the fix, and any backward compatibility or performance implications.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a condition to prevent an unhandled error in the search anywhere feature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the group/frontend Issue related to the frontend (React) label Mar 16, 2026
@gmazoyer gmazoyer changed the title Add small condition to prevent unhandled error Add small condition to prevent unhandled error for search anywhere Mar 16, 2026
@gmazoyer gmazoyer marked this pull request as ready for review March 16, 2026 10:23
@gmazoyer gmazoyer requested a review from a team as a code owner March 16, 2026 10:23
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/app/src/shared/api/rest/fetch.ts`:
- Around line 25-29: Add unit tests for fetchUrl to cover the new non-OK control
flow: (1) mock a non-OK Response whose body is JSON (e.g., {error: "msg"}) and
assert fetchUrl throws an Error containing/parsing that JSON error shape, and
(2) mock a non-OK Response whose body is non-JSON (e.g., plain text) and assert
fetchUrl falls back to a generic error message including the HTTP status; use
the same test harness/utilities used elsewhere to mock global fetch and import
the fetchUrl function to locate the behavior in fetch.ts.
- Around line 25-29: The non-OK branch in fetch.ts currently throws a plain
Error and discards the backend error payload; instead, read and parse the
response body (await rawResponse.json()) before throwing so the thrown value
preserves the backend error contract (e.g., include the parsed body and status),
then throw an object or Error that contains the parsed payload (so callers like
auth-callback.tsx can access error.errors) while keeping the existing status
check around rawResponse.ok and returning parsed JSON on success.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dfdefa3d-979f-4b5a-b596-d12743189812

📥 Commits

Reviewing files that changed from the base of the PR and between 06b064f and 41dd1e9.

📒 Files selected for processing (1)
  • frontend/app/src/shared/api/rest/fetch.ts

Comment on lines +25 to +29
if (!rawResponse.ok) {
throw new Error(`Request failed with status ${rawResponse.status}`);
}

return rawResponse.json();
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add tests for the new non-OK behavior of fetchUrl.

This introduces new control flow and error shape behavior, but no tests are included. Please add coverage for: (1) non-OK JSON error body and (2) non-OK non-JSON body fallback.

As per coding guidelines, "Write tests for new functionality in frontend code".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/src/shared/api/rest/fetch.ts` around lines 25 - 29, Add unit
tests for fetchUrl to cover the new non-OK control flow: (1) mock a non-OK
Response whose body is JSON (e.g., {error: "msg"}) and assert fetchUrl throws an
Error containing/parsing that JSON error shape, and (2) mock a non-OK Response
whose body is non-JSON (e.g., plain text) and assert fetchUrl falls back to a
generic error message including the HTTP status; use the same test
harness/utilities used elsewhere to mock global fetch and import the fetchUrl
function to locate the behavior in fetch.ts.

⚠️ Potential issue | 🟠 Major

Preserve error payload contract when throwing on non-OK responses.

This now throws a plain Error, but frontend/app/src/pages/auth-callback.tsx (Line 41) reads error.errors. That field is lost, so backend validation/auth errors won’t propagate correctly.

💡 Proposed fix (keep status check + preserve backend error details)
   if (!rawResponse.ok) {
-    throw new Error(`Request failed with status ${rawResponse.status}`);
+    const error = new Error(
+      `Request failed with status ${rawResponse.status}`
+    ) as Error & { status: number; errors?: unknown };
+    error.status = rawResponse.status;
+
+    try {
+      const errorBody = await rawResponse.json();
+      if (errorBody && typeof errorBody === "object" && "errors" in errorBody) {
+        error.errors = (errorBody as { errors?: unknown }).errors;
+      }
+    } catch {
+      // ignore non-JSON error response bodies
+    }
+
+    throw error;
   }

   return rawResponse.json();
📝 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
if (!rawResponse.ok) {
throw new Error(`Request failed with status ${rawResponse.status}`);
}
return rawResponse.json();
if (!rawResponse.ok) {
const error = new Error(
`Request failed with status ${rawResponse.status}`
) as Error & { status: number; errors?: unknown };
error.status = rawResponse.status;
try {
const errorBody = await rawResponse.json();
if (errorBody && typeof errorBody === "object" && "errors" in errorBody) {
error.errors = (errorBody as { errors?: unknown }).errors;
}
} catch {
// ignore non-JSON error response bodies
}
throw error;
}
return rawResponse.json();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/src/shared/api/rest/fetch.ts` around lines 25 - 29, The non-OK
branch in fetch.ts currently throws a plain Error and discards the backend error
payload; instead, read and parse the response body (await rawResponse.json())
before throwing so the thrown value preserves the backend error contract (e.g.,
include the parsed body and status), then throw an object or Error that contains
the parsed payload (so callers like auth-callback.tsx can access error.errors)
while keeping the existing status check around rawResponse.ok and returning
parsed JSON on success.

@gmazoyer gmazoyer force-pushed the gma-20260316-fix-search-without-docs branch from 41dd1e9 to 6a1cd81 Compare March 17, 2026 14:43
@gmazoyer gmazoyer force-pushed the gma-20260316-fix-search-without-docs branch from 6a1cd81 to 19e8fb6 Compare March 18, 2026 16:22
@gmazoyer gmazoyer merged commit 2af6114 into develop Mar 18, 2026
41 checks passed
@gmazoyer gmazoyer deleted the gma-20260316-fix-search-without-docs branch March 18, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/frontend Issue related to the frontend (React)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants