feat(eduaid_web): add API base fallback and backend connection status#512
Open
BrajamohanDas-afk wants to merge 4 commits intoAOSSIE-Org:mainfrom
Open
Conversation
📝 WalkthroughWalkthroughApp now subscribes to apiClient connection-status updates and renders a ConnectionBanner with a Retry action; apiClient gains listener support, fallback-aware request logic, JSON helpers, unified request flows, and explicit connection-status propagation (up/down/error). Changes
Sequence DiagramsequenceDiagram
participant App as App Component
participant APIClient as apiClient
participant Backend as Backend Server
App->>APIClient: subscribeConnectionStatus(listener)
note over APIClient: register listener for status updates
App->>APIClient: get/post/requestWithFallback(...)
APIClient->>Backend: request to primary baseUrl
alt Request succeeds
Backend-->>APIClient: 200 OK / JSON
APIClient->>APIClient: setConnectionStatus("up")
APIClient->>App: notifyConnectionStatus("up")
App->>App: update state, hide banner
else Network / fetch error
APIClient->>APIClient: try fallback baseUrls
alt fallback succeeds
Backend-->>APIClient: 200 OK via fallback
APIClient->>APIClient: update baseUrl, setConnectionStatus("up")
APIClient->>App: notifyConnectionStatus("up")
App->>App: update state, hide banner
else all attempts fail
APIClient->>APIClient: setConnectionStatus("down"/"error", detail)
APIClient->>App: notifyConnectionStatus("down"/"error", detail)
App->>App: update state, show ConnectionBanner
User->>App: click Retry
App->>App: reload page (trigger new requests)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🧹 Nitpick comments (2)
eduaid_web/src/App.css (1)
60-72: Add explicit keyboard focus styling for the Retry button.There is a hover style, but no explicit
:focus-visiblestyle. Adding one improves keyboard accessibility consistency.🎨 Suggested CSS addition
.connection-banner-retry:hover { filter: brightness(0.95); } + +.connection-banner-retry:focus-visible { + outline: 2px solid `#111827`; + outline-offset: 2px; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/App.css` around lines 60 - 72, The Retry button (.connection-banner-retry) lacks an explicit keyboard focus style; add a :focus-visible (and optionally :focus fallback) rule to mirror or complement the hover look with a clear visible focus indicator (e.g., a high-contrast outline or box-shadow and outline-offset) so keyboard users can see focus state consistently; update the CSS by adding a .connection-banner-retry:focus-visible selector (and .connection-banner-retry:focus for older browsers) that sets outline/box-shadow, outline-color, and outline-offset to match your design and ensure it doesn't conflict with the existing hover brightness rule.eduaid_web/src/App.js (1)
22-26: Prefer in-app retry over full page reload.Reloading resets route/state and can briefly hide connection context before another API call runs. A direct retry callback through
apiClientgives a smoother and more predictable recovery flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/App.js` around lines 22 - 26, The button handler currently calls window.location.reload() which forces a full page reload; change the onClick in App.js (the element with className "connection-banner-retry") to invoke an in-app retry routine instead — e.g., call a retry method on your API layer (apiClient.retryConnection or apiClient.retryPendingRequests) or dispatch a Redux/Context action (e.g., retryConnections or handleRetry) that replays/refreshes failed requests and updates connection state, so routes and UI state are preserved and the banner can reflect the retry outcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eduaid_web/src/utils/apiClient.js`:
- Around line 9-10: The code computes this.fallbackBaseUrls once in the
constructor (this.fallbackBaseUrls = this.getFallbackBaseUrls()), which lets
retries re-hit the same host after a baseUrl failover; change the failover/retry
logic so that whenever this.baseUrl is switched to a fallback host you also
recompute fallback targets by calling this.getFallbackBaseUrls() and assigning
the result to this.fallbackBaseUrls (update the same places that perform
failover/retries — e.g., the constructor and the failover/retry block around the
logic referenced in lines 77-83) so subsequent retries will use the updated set
of fallback URLs.
- Around line 103-113: The current flow sets this.setConnectionStatus("error",
...) for non-OK responses but the catch block always overwrites it to "down";
fix by marking HTTP errors before throwing and making the catch only set "down"
for non-HTTP/network errors: in the response.ok branch, create the Error, set a
flag like err.isHttpError = true (and call this.setConnectionStatus("error",
err.message)) and then throw err; in the catch block check if
(error.isHttpError) { throw error; } else { this.setConnectionStatus("down",
error.message); throw error; } so setConnectionStatus calls in response handling
are preserved (symbols: response.ok check, Error construction,
this.setConnectionStatus, catch(error)).
---
Nitpick comments:
In `@eduaid_web/src/App.css`:
- Around line 60-72: The Retry button (.connection-banner-retry) lacks an
explicit keyboard focus style; add a :focus-visible (and optionally :focus
fallback) rule to mirror or complement the hover look with a clear visible focus
indicator (e.g., a high-contrast outline or box-shadow and outline-offset) so
keyboard users can see focus state consistently; update the CSS by adding a
.connection-banner-retry:focus-visible selector (and
.connection-banner-retry:focus for older browsers) that sets outline/box-shadow,
outline-color, and outline-offset to match your design and ensure it doesn't
conflict with the existing hover brightness rule.
In `@eduaid_web/src/App.js`:
- Around line 22-26: The button handler currently calls window.location.reload()
which forces a full page reload; change the onClick in App.js (the element with
className "connection-banner-retry") to invoke an in-app retry routine instead —
e.g., call a retry method on your API layer (apiClient.retryConnection or
apiClient.retryPendingRequests) or dispatch a Redux/Context action (e.g.,
retryConnections or handleRetry) that replays/refreshes failed requests and
updates connection state, so routes and UI state are preserved and the banner
can reflect the retry outcome.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
eduaid_web/src/utils/apiClient.js (1)
142-165: Consider deduplicating FormData status/error handling paths.
postFormDatanow has a separate Electron branch with overlapping status logic. Extracting a shared helper would reduce drift risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/utils/apiClient.js` around lines 142 - 165, postFormData currently duplicates connection-status and error handling between the isElectron branch and the fallback path; extract a shared helper (e.g., performFormPost or handleFormPostResponse) that accepts (endpoint, formData, options) and encapsulates the fetch/request, calls parseJsonResponse, and invokes setConnectionStatus("up") on success or setConnectionStatus(status, error.message) on failure; then have postFormData call that helper in both the isElectron branch (using fetch to this.baseUrl + endpoint inside the helper when isElectron is true) and the requestWithFallback branch (have the helper optionally call requestWithFallback or accept a requester function), referencing postFormData, requestWithFallback, parseJsonResponse, setConnectionStatus and isElectron to locate the relevant logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eduaid_web/src/utils/apiClient.js`:
- Around line 14-17: subscribeConnectionStatus currently just registers
listeners and returns an unsubscribe, so new subscribers miss the current
connection status; modify subscribeConnectionStatus to immediately invoke the
provided listener with the current connection state (e.g., call
listener(this.connectionState)) before adding it to this.listeners so late
subscribers receive the present state (and still return the same unsubscribe
function that removes from this.listeners).
---
Nitpick comments:
In `@eduaid_web/src/utils/apiClient.js`:
- Around line 142-165: postFormData currently duplicates connection-status and
error handling between the isElectron branch and the fallback path; extract a
shared helper (e.g., performFormPost or handleFormPostResponse) that accepts
(endpoint, formData, options) and encapsulates the fetch/request, calls
parseJsonResponse, and invokes setConnectionStatus("up") on success or
setConnectionStatus(status, error.message) on failure; then have postFormData
call that helper in both the isElectron branch (using fetch to this.baseUrl +
endpoint inside the helper when isElectron is true) and the requestWithFallback
branch (have the helper optionally call requestWithFallback or accept a
requester function), referencing postFormData, requestWithFallback,
parseJsonResponse, setConnectionStatus and isElectron to locate the relevant
logic.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eduaid_web/src/utils/apiClient.js`:
- Around line 49-55: parseJsonResponse currently calls response.json() for any
successful response, which throws on empty bodies (e.g., 204/205) and surfaces
as an error; update parseJsonResponse to detect empty-success responses before
parsing (check response.status === 204 || response.status === 205 or check
headers like response.headers.get('Content-Length') === '0' or
response.headers.get('Content-Type') is missing/empty) and return null (or an
appropriate empty value) for those cases; otherwise call and return await
response.json() as before and keep the existing error branch that reads
response.text() and throws an Error including status and text. Ensure you update
the function parseJsonResponse to perform the empty-body check prior to calling
response.json().
- Around line 14-17: subscribeConnectionStatus currently calls the listener
directly during subscription which can throw and break the subscription; change
it to wrap the initial emission in the same safe-call pattern used by
notifyConnectionStatus: after adding the listener to this.listeners, invoke the
listener inside a try/catch (or the same helper used by notifyConnectionStatus)
so any exception is caught and logged (or ignored) without preventing the
returned unsubscribe function from being used; keep the listener
registration/removal behavior (this.listeners.add(listener) and return () =>
this.listeners.delete(listener)) unchanged.
- Around line 143-166: postFormData currently bypasses the unified IPC path;
update the IPC handler (the channel used by window.electronAPI.makeApiRequest)
to detect when body is a FormData, serialize it into a multipart/form-data
payload with a generated boundary (or encode parts as ArrayBuffers/Blobs and
send boundary/value metadata so the main process can reconstruct the multipart
body), and accept a marker indicating "isFormData" so the handler sets the
proper Content-Type with boundary and forwards the raw multipart bytes to fetch;
then change postFormData to call the same IPC method (or requestWithFallback) as
other requests (window.electronAPI.makeApiRequest / requestWithFallback) so it
receives the normalized error shape (including isHttpError), uses the same
connection-status logic (setConnectionStatus with normalized error), and
benefits from the fallback logic in requestWithFallback. Ensure you reference
and preserve symbols postFormData, requestWithFallback,
window.electronAPI.makeApiRequest, and any normalized error fields (isHttpError)
when implementing.
Comment on lines
143
to
166
| async postFormData(endpoint, formData, options = {}) { | ||
| if (this.isElectron) { | ||
| // For Electron, we need to handle file uploads differently | ||
| // Since we can't easily pass files through IPC, we'll fall back to fetch | ||
| const url = `${this.baseUrl}${endpoint}`; | ||
| const response = await fetch(url, { | ||
| ...options, | ||
| method: 'POST', | ||
| body: formData | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| } | ||
|
|
||
| return await response.json(); | ||
| } else { | ||
| // For web, use FormData directly | ||
| const url = `${this.baseUrl}${endpoint}`; | ||
| const response = await fetch(url, { | ||
| ...options, | ||
| method: 'POST', | ||
| body: formData | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| try { | ||
| const response = await fetch(`${this.baseUrl}${endpoint}`, { | ||
| ...options, | ||
| method: "POST", | ||
| body: formData, | ||
| }); | ||
| const data = await this.parseJsonResponse(response); | ||
| this.setConnectionStatus("up"); | ||
| return data; | ||
| } catch (error) { | ||
| const status = error instanceof TypeError ? "down" : "error"; | ||
| this.setConnectionStatus(status, error.message); | ||
| throw error; | ||
| } | ||
|
|
||
| return await response.json(); | ||
| } | ||
|
|
||
| return this.requestWithFallback(endpoint, { | ||
| ...options, | ||
| method: "POST", | ||
| body: formData, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, find the apiClient.js file and understand the repository structure
find . -name "apiClient.js" -o -name "*apiClient*" -o -name "*electron*" | head -20Repository: AOSSIE-Org/EduAid
Length of output: 96
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for makeApiRequest usage and electronAPI references
rg -n 'makeApiRequest|electronAPI|contextBridge|ipcRenderer' --type js --type ts --type jsx --type tsx -B 2 -A 2Repository: AOSSIE-Org/EduAid
Length of output: 88
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find preload scripts and main process files
fd -HI 'preload|main\.js|ipc' | grep -E '\.(js|ts)$'Repository: AOSSIE-Org/EduAid
Length of output: 109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Examine the full apiClient.js file to understand all request methods
cat -n eduaid_web/src/utils/apiClient.js | head -200Repository: AOSSIE-Org/EduAid
Length of output: 6336
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Examine preload.js to see how makeApiRequest is exposed
cat -n eduaid_desktop/preload.jsRepository: AOSSIE-Org/EduAid
Length of output: 2395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Examine main.js to see the IPC handler for makeApiRequest
cat -n eduaid_desktop/main.js | head -300Repository: AOSSIE-Org/EduAid
Length of output: 8621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for auth, token, session, headers handling in main.js and config
rg -n 'auth|token|session|Authorization|headers' eduaid_desktop/ -A 2 -B 1Repository: AOSSIE-Org/EduAid
Length of output: 1286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check config.js to understand the setup
find eduaid_desktop -name "config.js" -o -name "config.ts" -exec cat -n {} \;Repository: AOSSIE-Org/EduAid
Length of output: 43
🏁 Script executed:
#!/bin/bash
set -eulo pipefail
# Search for any FormData usage in the codebase
rg -n 'FormData|multipart' --type js --type ts -A 3 -B 1Repository: AOSSIE-Org/EduAid
Length of output: 1554
Enhance IPC handler to support FormData and unify with direct fetch path.
postFormData() uses direct fetch in Electron while other requests use window.electronAPI.makeApiRequest. This split exists because the current IPC handler hardcodes 'Content-Type': 'application/json' and cannot serialize FormData objects.
The workaround causes inconsistencies:
- Error handling differs:
postFormDatacheckserror instanceof TypeError, whilemakeRequestuses anisHttpErrorflag postFormDatalacks the normalized error response structurepostFormDatahas no fallback mechanism (unlikemakeRequest)- Potential missing auth/session handling if added to IPC layer later
Extend the IPC handler to support multipart/form-data with proper boundary generation, then route postFormData through the unified makeApiRequest channel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eduaid_web/src/utils/apiClient.js` around lines 143 - 166, postFormData
currently bypasses the unified IPC path; update the IPC handler (the channel
used by window.electronAPI.makeApiRequest) to detect when body is a FormData,
serialize it into a multipart/form-data payload with a generated boundary (or
encode parts as ArrayBuffers/Blobs and send boundary/value metadata so the main
process can reconstruct the multipart body), and accept a marker indicating
"isFormData" so the handler sets the proper Content-Type with boundary and
forwards the raw multipart bytes to fetch; then change postFormData to call the
same IPC method (or requestWithFallback) as other requests
(window.electronAPI.makeApiRequest / requestWithFallback) so it receives the
normalized error shape (including isHttpError), uses the same connection-status
logic (setConnectionStatus with normalized error), and benefits from the
fallback logic in requestWithFallback. Ensure you reference and preserve symbols
postFormData, requestWithFallback, window.electronAPI.makeApiRequest, and any
normalized error fields (isHttpError) when implementing.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
eduaid_web/src/utils/apiClient.js (2)
41-51: Consider preserving the hostname when computing fallback URLs.The fallback URLs are hardcoded to
localhosteven whenthis.baseUrluses127.0.0.1. This could cause issues if the server binds to a specific address.Suggested improvement
getFallbackBaseUrls() { if (this.isElectron || process.env.REACT_APP_BASE_URL) return []; if (typeof window === "undefined") return []; const host = window.location.hostname; if (host !== "localhost" && host !== "127.0.0.1") return []; - if (this.baseUrl.includes(":5000")) return ["http://localhost:5001"]; - if (this.baseUrl.includes(":5001")) return ["http://localhost:5000"]; + if (this.baseUrl.includes(":5000")) return [`http://${host}:5001`]; + if (this.baseUrl.includes(":5001")) return [`http://${host}:5000`]; return []; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/utils/apiClient.js` around lines 41 - 51, The getFallbackBaseUrls method currently returns hardcoded "http://localhost:5000/5001" addresses; update it to preserve the original hostname by using the local variable host (from window.location.hostname) when constructing fallback URLs so that if this.baseUrl contains ":5000" you return `http://${host}:5001` and if it contains ":5001" return `http://${host}:5000`; keep the existing early exits (this.isElectron, REACT_APP_BASE_URL, typeof window === "undefined", and non-localhost checks) and reference getFallbackBaseUrls, this.baseUrl, and host when making the change.
53-64: Addawaitbeforeresponse.json()for consistency.While functionally equivalent in this context, the missing
awaiton line 63 is inconsistent with other async operations in this function and can affect stack traces when debugging JSON parse errors.Suggested fix
- return response.json(); + return await response.json();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/utils/apiClient.js` around lines 53 - 64, In parseJsonResponse, add an await before the call to response.json() so the function returns the resolved JSON value (e.g., return await response.json()); update the return at the end of parseJsonResponse to use await on response.json() to keep async handling consistent and preserve proper stack traces when JSON parsing fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@eduaid_web/src/utils/apiClient.js`:
- Around line 41-51: The getFallbackBaseUrls method currently returns hardcoded
"http://localhost:5000/5001" addresses; update it to preserve the original
hostname by using the local variable host (from window.location.hostname) when
constructing fallback URLs so that if this.baseUrl contains ":5000" you return
`http://${host}:5001` and if it contains ":5001" return `http://${host}:5000`;
keep the existing early exits (this.isElectron, REACT_APP_BASE_URL, typeof
window === "undefined", and non-localhost checks) and reference
getFallbackBaseUrls, this.baseUrl, and host when making the change.
- Around line 53-64: In parseJsonResponse, add an await before the call to
response.json() so the function returns the resolved JSON value (e.g., return
await response.json()); update the return at the end of parseJsonResponse to use
await on response.json() to keep async handling consistent and preserve proper
stack traces when JSON parsing fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Fixes #509
Additional Notes:
apiClient(unknown | up | down | error).localhost:5000tolocalhost:5001(and reverse) whenREACT_APP_BASE_URLis not set.App.js:downstate: backend unreachable + Retry buttonerrorstate: backend reachable but request failed (4xx/5xx)npm run build(build succeeds; existing repo warnings remain).Checklist
AI Usage Disclosure
I have used the following AI models and tools: Codex (GPT-5) in VS Code chat.
Summary by CodeRabbit