Improve Frontend Resilience: Error Boundaries, Timeout Handling, and …#567
Open
zohaib-7035 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Open
Improve Frontend Resilience: Error Boundaries, Timeout Handling, and …#567zohaib-7035 wants to merge 1 commit intoAOSSIE-Org:mainfrom
zohaib-7035 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
…Safe Data Validation Resolves Issue AOSSIE-Org#469: Infinite loading loops during backend timeouts. Resolves Issue AOSSIE-Org#467: Application crashes due to malformed data.
📝 WalkthroughWalkthroughThe pull request introduces ESLint configuration, five new React components (EmptyState, ErrorBoundary, QuizRenderer, TranscriptViewer), a custom fetch hook with timeout and error handling, a data normalization utility, and refactors two existing pages to integrate these new utilities with improved error resilience. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as React Component
participant useAIFetch as useAIFetch Hook
participant Backend as AI Backend
participant Normalize as normalizeArrayData
participant Render as Render Layer
Component->>useAIFetch: Call hook with url & timeout
useAIFetch->>Backend: fetch(url, fetchOptions)
alt Timeout
Backend-->>useAIFetch: (AbortController triggered)
useAIFetch->>useAIFetch: error = TIMEOUT
else Network Error
Backend-->>useAIFetch: Network failure
useAIFetch->>useAIFetch: error = NETWORK
else Success (non-OK)
Backend-->>useAIFetch: HTTP error response
useAIFetch->>useAIFetch: error = NETWORK
else Success (OK)
Backend-->>useAIFetch: { data: [...] }
useAIFetch->>useAIFetch: isLoading = false
end
useAIFetch->>Component: Return { data, isLoading, error }
Component->>Normalize: normalizeArrayData(data)
Normalize->>Normalize: Ensure array shape
Normalize-->>Component: Normalized array or []
Component->>Render: Render with ErrorBoundary
alt error exists
Render->>Render: Show EmptyState (error message)
else isLoading
Render->>Render: Show loading spinner
else data empty
Render->>Render: Show EmptyState (no data)
else data available
Render->>Render: Map & render items
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can disable poems in the walkthrough.Disable the |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
extension/src/pages/question/SidePanel.jsx (1)
63-69:⚠️ Potential issue | 🟠 MajorGuard
qaPair.answerbefore using array methods.Line 64-69 calls
.filter()and.find()onqaPair.answerwithout type checks. Malformed payloads will throw and break quiz parsing.💡 Proposed fix
- const options = qaPair.answer - .filter((ans) => !ans.correct) - .map((ans) => ans.answer); - const correctAnswer = qaPair.answer.find( - (ans) => ans.correct - )?.answer; + const answers = normalizeArrayData(qaPair.answer).filter( + (ans) => ans && typeof ans === "object" + ); + const options = answers + .filter((ans) => !ans.correct) + .map((ans) => ans.answer); + const correctAnswer = answers.find((ans) => ans.correct)?.answer;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/src/pages/question/SidePanel.jsx` around lines 63 - 69, The code calls array methods on qaPair.answer without validating it, which can throw for malformed payloads; update the processing in SidePanel.jsx (around normalizeArrayData/qaPairsFromStorage handling) to guard qaPair.answer by checking Array.isArray(qaPair.answer) (or coerce with const answers = Array.isArray(qaPair.answer) ? qaPair.answer : []), then use answers.filter(...) and answers.find(...) (and fall back to sensible defaults for options and correctAnswer) so parsing never crashes when qaPair.answer is missing or not an array.
🧹 Nitpick comments (8)
extension/src/components/TranscriptViewer.jsx (2)
42-46: Consider using a stable key if segments have unique identifiers.Using
indexas a key works for static lists but can cause issues if segments are reordered or filtered. Ifsegmentobjects have a uniqueidortimestamp, prefer using that.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/src/components/TranscriptViewer.jsx` around lines 42 - 46, The map in TranscriptViewer.jsx uses the array index as the React key which can break identity when segments are reordered or filtered; change the key in the transcriptSegments.map callback to use a stable unique identifier from each segment (e.g., segment.id or segment.timestamp) instead of index so React can correctly preserve element state during updates.
12-20: Parameterize the URL and request body for reusability.The component has hardcoded
urlandvideo_urlvalues. For production use, these should be passed as props to make the component reusable.♻️ Proposed parameterization
-export default function TranscriptViewer() { - // Note: Ensure the URL matches a valid backend endpoint for this example. - const { data, isLoading, error } = useAIFetch({ - url: "http://localhost:5000/get_transcript", // Just a demo URL - fetchOptions: { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ video_url: "example" }), - }, - timeout: 45000, // 45s timeout for transcript - }); +export default function TranscriptViewer({ videoUrl, apiEndpoint = "http://localhost:5000/get_transcript", timeout = 45000 }) { + const { data, isLoading, error } = useAIFetch({ + url: apiEndpoint, + fetchOptions: { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ video_url: videoUrl }), + }, + timeout, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/src/components/TranscriptViewer.jsx` around lines 12 - 20, The component currently hardcodes the request URL and body in the useAIFetch call; make the TranscriptViewer component accept props (e.g., transcriptUrl and videoUrl) and replace the hardcoded url and body values in the useAIFetch invocation (url -> transcriptUrl, body -> JSON.stringify({ video_url: videoUrl })); add sensible defaults or prop-type checks for transcriptUrl and videoUrl so the component remains reusable and robust, and ensure timeout stays unchanged unless made configurable via a prop.extension/src/components/EmptyState.jsx (1)
1-2: Minor: Remove leading blank lines.The file starts with two empty lines. While not a functional issue, removing them improves consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/src/components/EmptyState.jsx` around lines 1 - 2, The file extension/src/components/EmptyState.jsx has two leading blank lines before the EmptyState component; remove those blank lines so the file starts directly with the component declaration (e.g., the EmptyState component/export) to improve consistency and formatting.extension/src/hooks/useAIFetch.js (1)
75-77: HTTP errors are classified as NETWORK, which may be misleading.Non-OK HTTP responses (e.g., 400, 404, 500) are thrown and caught as
NETWORKerrors. Consider distinguishing HTTP errors from actual network failures for better user messaging.♻️ Proposed error classification enhancement
+/** `@typedef` {"TIMEOUT" | "NETWORK" | "HTTP_ERROR"} AIFetchErrorType */ if (!response.ok) { - throw new Error(`HTTP ${response.status}`); + const httpError = new Error(`HTTP ${response.status}`); + httpError.name = "HTTPError"; + httpError.status = response.status; + throw httpError; } // ... } catch (err) { if (err.name === "AbortError") { setError("TIMEOUT"); + } else if (err.name === "HTTPError") { + setError("HTTP_ERROR"); } else { setError("NETWORK"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/src/hooks/useAIFetch.js` around lines 75 - 77, The current throw inside useAIFetch.js uses new Error(`HTTP ${response.status}`) which gets classified as a NETWORK error; change this to throw a distinct HTTP error (e.g., create or instantiate an error with a type or name of "HTTP" and include the HTTP status and optionally response body) so callers can distinguish HTTP failures from real network failures; update the throw site in the useAIFetch function (where it checks response.ok) to attach error.type = 'HTTP' or error.name = 'HTTPError' and include response.status/responseText, and ensure downstream catch logic inspects that type/name to present correct user messaging.extension/eslint.config.js (1)
14-18: Consider enabling React plugin recommended rules.The
reactplugin is imported but no React-specific rules are enabled. Without rules likereact/jsx-uses-vars, ESLint may report false positives for JSX component imports as unused.♻️ Proposed addition of React rules
rules: { "no-unused-vars": "warn", "react-hooks/rules-of-hooks": "error", - "react-hooks/exhaustive-deps": "warn" + "react-hooks/exhaustive-deps": "warn", + "react/jsx-uses-react": "off", // Not needed with React 17+ JSX transform + "react/react-in-jsx-scope": "off", // Not needed with React 17+ JSX transform + "react/jsx-uses-vars": "error", // Prevents false unused-var warnings for JSX },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/eslint.config.js` around lines 14 - 18, The rules block in eslint.config.js currently lacks React-specific rules even though the react plugin is included, causing false positives for JSX imports; update the rules (in the rules object where "no-unused-vars", "react-hooks/rules-of-hooks", and "react-hooks/exhaustive-deps" are defined) to enable the React recommended rule set or at minimum add rules such as "react/jsx-uses-vars" and "react/jsx-uses-react" (or enable "plugin:react/recommended") so JSX component imports are treated as used by ESLint.extension/src/components/QuizRenderer.jsx (1)
17-25: Parameterize the URL and request body for reusability.Similar to
TranscriptViewer, this component has hardcoded values. Consider accepting props forapiEndpoint,inputText,maxQuestions, andtimeoutto make the component reusable across different contexts.♻️ Proposed parameterization
-export default function QuizRenderer() { - // Simulate fetch call. The URL should match your backend endpoint. - const { data, isLoading, error } = useAIFetch({ - url: "http://localhost:5000/get_mcq", // Ensure this endpoint exists - fetchOptions: { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ input_text: "Sample Context Layer", max_questions: 2 }), - }, - timeout: 30000, - }); +export default function QuizRenderer({ + inputText, + maxQuestions = 2, + apiEndpoint = "http://localhost:5000/get_mcq", + timeout = 30000 +}) { + const { data, isLoading, error } = useAIFetch({ + url: apiEndpoint, + fetchOptions: { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ input_text: inputText, max_questions: maxQuestions }), + }, + timeout, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/src/components/QuizRenderer.jsx` around lines 17 - 25, Replace the hardcoded fetch parameters in the useAIFetch call inside QuizRenderer (the component in QuizRenderer.jsx) with props so the component is reusable: add props apiEndpoint, inputText, maxQuestions, and timeout to the QuizRenderer component signature and use them to populate url, body (JSON.stringify({ input_text: inputText, max_questions: maxQuestions })), and timeout passed to useAIFetch; keep sensible defaults via defaultProps or default parameter values so existing consumers aren't broken.extension/src/components/ErrorBoundary.jsx (1)
14-47: Consider adding a reset mechanism for error recovery.The ErrorBoundary catches errors correctly, but once
hasErroris true, users are stuck on the fallback UI with no way to retry. This limits usability, especially for transient render errors.A common pattern is to provide a
resetErrorBoundarycallback or use akeyprop to allow re-mounting.♻️ Proposed enhancement with reset capability
class ErrorBoundary extends React.Component { constructor(props) { super(props); this.state = { hasError: false }; + this.resetErrorBoundary = this.resetErrorBoundary.bind(this); } + + resetErrorBoundary() { + this.setState({ hasError: false }); + } static getDerivedStateFromError() { return { hasError: true }; } componentDidCatch(error, errorInfo) { console.error("ErrorBoundary caught error:", error, errorInfo); } render() { if (this.state.hasError) { - return this.props.fallback; + // Clone fallback to inject resetErrorBoundary if it accepts the prop + return typeof this.props.fallback === 'function' + ? this.props.fallback({ resetErrorBoundary: this.resetErrorBoundary }) + : this.props.fallback; } return this.props.children; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/src/components/ErrorBoundary.jsx` around lines 14 - 47, The ErrorBoundary needs a reset mechanism so users can recover from the fallback UI; add a resetErrorBoundary instance method (and track error/errorInfo in state) that sets hasError back to false, update the constructor to initialize and bind/reset state, and modify render to accept a function fallback (i.e. if typeof this.props.fallback === 'function' call this.props.fallback({ resetErrorBoundary })) so callers can render a retry button that calls resetErrorBoundary; keep existing getDerivedStateFromError and componentDidCatch but ensure componentDidCatch also stores error/errorInfo into state for potential display or logging.extension/package.json (1)
6-10: Consider adding a lint script.ESLint dependencies were added but no
lintscript is defined, making it harder for developers to run linting consistently.♻️ Proposed lint script addition
"scripts": { "dev": "vite", "build": "vite build", - "preview": "vite preview" + "preview": "vite preview", + "lint": "eslint src/" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/package.json` around lines 6 - 10, The scripts section in package.json (where "dev","build","preview" live) lacks a lint entry; add a "lint" npm script that runs the repository ESLint configuration over the extension source files (and optionally a "lint:fix" script to auto-fix), so contributors can run linting consistently from package.json using the existing ESLint dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extension/package.json`:
- Around line 25-28: Update the dependency versions in package.json: change
"eslint-plugin-react" from "^7.37.5" to the existing release "^7.37.4" and
replace "eslint-plugin-react-hooks" "^7.0.1" with either a known stable release
(choose the latest stable semver for eslint-plugin-react-hooks) or, if a canary
is intentional, pin the exact canary tag/version you want; ensure both package
names and updated versions are applied in the dependencies block so npm/yarn
installs valid releases.
In `@extension/src/components/QuizRenderer.jsx`:
- Line 44: Fix the typo in the user-facing message used when no quizzes are
returned: in QuizRenderer.jsx update the message prop passed to the EmptyState
component (where content is set) from "No quizzes could be parse from the
response. Please try again." to "No quizzes could be parsed from the response.
Please try again." so the EmptyState message reads correctly; search for the
assignment to the variable content that renders <EmptyState message=... /> to
locate the exact spot.
In `@extension/src/hooks/useAIFetch.js`:
- Around line 109-110: The hook useAIFetch currently omits fetchOptions from the
dependency array (only [url, timeout]), which can cause stale-closure bugs when
fetchOptions (for example body) changes; update useAIFetch to include a stable
representation of fetchOptions in the deps—either accept and document that
callers must pass a memoized fetchOptions, or add fetchOptions to the dependency
list using a stable comparator such as JSON.stringify(fetchOptions) (or require
callers to use useMemo) so the effect re-runs when options meaningfully change;
reference the useAIFetch effect dependencies and the fetchOptions variable when
making this change.
In `@extension/src/pages/question/Question.jsx`:
- Around line 1-5: The file references ErrorBoundary and EmptyState but never
imports them; add import statements for ErrorBoundary and EmptyState at the top
of the file alongside the other imports so the JSX at lines where ErrorBoundary
and EmptyState are used can resolve; ensure you import the correct exported
names (ErrorBoundary, EmptyState) from their component modules and place the
imports with the existing React/component imports.
- Line 180: Replace the typo in the EmptyState message prop used in
Question.jsx: change the string passed to the EmptyState component (message="No
quizzes could be parse from the response. Please try again.") to use the correct
past participle ("parsed") so it reads "No quizzes could be parsed from the
response. Please try again." Ensure you update the message prop on the
EmptyState JSX element.
- Line 280: The Question component is exported but never mounted to the DOM;
update Question.jsx to mount the component after its definition by invoking
ReactDOM.render with <Question /> into the page root
(document.getElementById("root")) like Answer.jsx does, ensuring ReactDOM is
imported if not already and keeping the existing export default Question; this
will bootstrap the module script and render the component on question.html.
In `@extension/src/pages/question/SidePanel.jsx`:
- Around line 1-5: The component references ErrorBoundary and EmptyState at
render (used around the JSX block where they wrap content, lines ~225-229) but
they are not imported; add proper imports at the top of SidePanel.jsx (for
example: import ErrorBoundary from '...'; import EmptyState from '...') matching
your project module paths or relative locations so the ErrorBoundary and
EmptyState symbols are defined before use in the SidePanel component; ensure the
import names match the exported identifiers and save.
- Line 228: Update the grammatically incorrect empty-state message in
SidePanel.jsx where the EmptyState component is rendered; change the message
prop value from "No quizzes could be parse from the response. Please try again."
to "No quizzes could be parsed from the response. Please try again." so the word
"parse" is corrected to "parsed."
- Around line 246-269: The rendering currently uses a truthy check
(qaPair.options && ...) which treats an empty array as truthy so the
multiple-choice block renders nothing and the text-input fallback is skipped;
change the condition to explicitly check for a non-empty array (e.g.,
Array.isArray(qaPair.options) && qaPair.options.length > 0) for the
shuffledOptions mapping and render the text-input fallback when there are no
options; also ensure shuffledOptions is only used/constructed when
qaPair.options is a non-empty array to avoid mapping over undefined.
- Line 304: The SidePanel component is exported but never mounted into the DOM;
import the React DOM renderer and render SidePanel into the page's root element
(document.getElementById('root') or '#root') at the end of the file so the
component actually mounts; specifically, add the appropriate React DOM import
(react-dom/client createRoot for React 18 or react-dom render for older
versions) and call createRoot(...).render(<SidePanel />) or
ReactDOM.render(<SidePanel />, root) after the SidePanel export.
---
Outside diff comments:
In `@extension/src/pages/question/SidePanel.jsx`:
- Around line 63-69: The code calls array methods on qaPair.answer without
validating it, which can throw for malformed payloads; update the processing in
SidePanel.jsx (around normalizeArrayData/qaPairsFromStorage handling) to guard
qaPair.answer by checking Array.isArray(qaPair.answer) (or coerce with const
answers = Array.isArray(qaPair.answer) ? qaPair.answer : []), then use
answers.filter(...) and answers.find(...) (and fall back to sensible defaults
for options and correctAnswer) so parsing never crashes when qaPair.answer is
missing or not an array.
---
Nitpick comments:
In `@extension/eslint.config.js`:
- Around line 14-18: The rules block in eslint.config.js currently lacks
React-specific rules even though the react plugin is included, causing false
positives for JSX imports; update the rules (in the rules object where
"no-unused-vars", "react-hooks/rules-of-hooks", and
"react-hooks/exhaustive-deps" are defined) to enable the React recommended rule
set or at minimum add rules such as "react/jsx-uses-vars" and
"react/jsx-uses-react" (or enable "plugin:react/recommended") so JSX component
imports are treated as used by ESLint.
In `@extension/package.json`:
- Around line 6-10: The scripts section in package.json (where
"dev","build","preview" live) lacks a lint entry; add a "lint" npm script that
runs the repository ESLint configuration over the extension source files (and
optionally a "lint:fix" script to auto-fix), so contributors can run linting
consistently from package.json using the existing ESLint dependencies.
In `@extension/src/components/EmptyState.jsx`:
- Around line 1-2: The file extension/src/components/EmptyState.jsx has two
leading blank lines before the EmptyState component; remove those blank lines so
the file starts directly with the component declaration (e.g., the EmptyState
component/export) to improve consistency and formatting.
In `@extension/src/components/ErrorBoundary.jsx`:
- Around line 14-47: The ErrorBoundary needs a reset mechanism so users can
recover from the fallback UI; add a resetErrorBoundary instance method (and
track error/errorInfo in state) that sets hasError back to false, update the
constructor to initialize and bind/reset state, and modify render to accept a
function fallback (i.e. if typeof this.props.fallback === 'function' call
this.props.fallback({ resetErrorBoundary })) so callers can render a retry
button that calls resetErrorBoundary; keep existing getDerivedStateFromError and
componentDidCatch but ensure componentDidCatch also stores error/errorInfo into
state for potential display or logging.
In `@extension/src/components/QuizRenderer.jsx`:
- Around line 17-25: Replace the hardcoded fetch parameters in the useAIFetch
call inside QuizRenderer (the component in QuizRenderer.jsx) with props so the
component is reusable: add props apiEndpoint, inputText, maxQuestions, and
timeout to the QuizRenderer component signature and use them to populate url,
body (JSON.stringify({ input_text: inputText, max_questions: maxQuestions })),
and timeout passed to useAIFetch; keep sensible defaults via defaultProps or
default parameter values so existing consumers aren't broken.
In `@extension/src/components/TranscriptViewer.jsx`:
- Around line 42-46: The map in TranscriptViewer.jsx uses the array index as the
React key which can break identity when segments are reordered or filtered;
change the key in the transcriptSegments.map callback to use a stable unique
identifier from each segment (e.g., segment.id or segment.timestamp) instead of
index so React can correctly preserve element state during updates.
- Around line 12-20: The component currently hardcodes the request URL and body
in the useAIFetch call; make the TranscriptViewer component accept props (e.g.,
transcriptUrl and videoUrl) and replace the hardcoded url and body values in the
useAIFetch invocation (url -> transcriptUrl, body -> JSON.stringify({ video_url:
videoUrl })); add sensible defaults or prop-type checks for transcriptUrl and
videoUrl so the component remains reusable and robust, and ensure timeout stays
unchanged unless made configurable via a prop.
In `@extension/src/hooks/useAIFetch.js`:
- Around line 75-77: The current throw inside useAIFetch.js uses new Error(`HTTP
${response.status}`) which gets classified as a NETWORK error; change this to
throw a distinct HTTP error (e.g., create or instantiate an error with a type or
name of "HTTP" and include the HTTP status and optionally response body) so
callers can distinguish HTTP failures from real network failures; update the
throw site in the useAIFetch function (where it checks response.ok) to attach
error.type = 'HTTP' or error.name = 'HTTPError' and include
response.status/responseText, and ensure downstream catch logic inspects that
type/name to present correct user messaging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab4a39ec-bf5e-4b77-81e0-0f9916ffee6e
⛔ Files ignored due to path filters (1)
extension/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
extension/eslint.config.jsextension/package.jsonextension/src/components/EmptyState.jsxextension/src/components/ErrorBoundary.jsxextension/src/components/QuizRenderer.jsxextension/src/components/TranscriptViewer.jsxextension/src/hooks/useAIFetch.jsextension/src/pages/question/Question.jsxextension/src/pages/question/SidePanel.jsxextension/src/utils/normalizeArrayData.js
Comment on lines
+25
to
+28
| "eslint": "^9.39.4", | ||
| "eslint-plugin-react": "^7.37.5", | ||
| "eslint-plugin-react-hooks": "^7.0.1", | ||
| "globals": "^17.4.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that the specified ESLint package versions exist on npm
echo "Checking eslint versions..."
npm view eslint versions --json 2>/dev/null | jq -r '.[]' | grep -E "^9\.39\." | head -5
echo -e "\nChecking eslint-plugin-react versions..."
npm view eslint-plugin-react versions --json 2>/dev/null | jq -r '.[]' | grep -E "^7\.37\." | head -5
echo -e "\nChecking eslint-plugin-react-hooks versions..."
npm view eslint-plugin-react-hooks versions --json 2>/dev/null | jq -r '.[]' | grep -E "^7\.0\." | head -5
echo -e "\nChecking globals versions..."
npm view globals versions --json 2>/dev/null | jq -r '.[]' | grep -E "^17\." | head -5Repository: AOSSIE-Org/EduAid
Length of output: 468
Fix non-existent package versions in package.json.
eslint-plugin-react@^7.37.5 does not exist (latest is 7.37.4), and eslint-plugin-react-hooks@^7.0.1 has no stable release (only pre-release canary builds). Update to:
eslint-plugin-react@^7.37.4eslint-plugin-react-hooksto an available stable version or pin a specific canary release if that is intentional
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/package.json` around lines 25 - 28, Update the dependency versions
in package.json: change "eslint-plugin-react" from "^7.37.5" to the existing
release "^7.37.4" and replace "eslint-plugin-react-hooks" "^7.0.1" with either a
known stable release (choose the latest stable semver for
eslint-plugin-react-hooks) or, if a canary is intentional, pin the exact canary
tag/version you want; ensure both package names and updated versions are applied
in the dependencies block so npm/yarn installs valid releases.
| } else if (error === "NETWORK") { | ||
| content = <EmptyState message="Failed to connect to the server. Please check your connection." />; | ||
| } else if (quizzes.length === 0) { | ||
| content = <EmptyState message="No quizzes could be parse from the response. Please try again." />; |
There was a problem hiding this comment.
Typo in user-facing message: "parse" should be "parsed".
The message reads "No quizzes could be parse from the response" instead of "parsed".
✏️ Proposed fix
- content = <EmptyState message="No quizzes could be parse from the response. Please try again." />;
+ content = <EmptyState message="No quizzes could be parsed from the response. 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.
Suggested change
| content = <EmptyState message="No quizzes could be parse from the response. Please try again." />; | |
| content = <EmptyState message="No quizzes could be parsed from the response. Please try again." />; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/components/QuizRenderer.jsx` at line 44, Fix the typo in the
user-facing message used when no quizzes are returned: in QuizRenderer.jsx
update the message prop passed to the EmptyState component (where content is
set) from "No quizzes could be parse from the response. Please try again." to
"No quizzes could be parsed from the response. Please try again." so the
EmptyState message reads correctly; search for the assignment to the variable
content that renders <EmptyState message=... /> to locate the exact spot.
Comment on lines
+109
to
+110
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [url, timeout]); |
There was a problem hiding this comment.
Excluding fetchOptions from dependencies may cause stale closure issues.
If fetchOptions (e.g., body content) changes between renders, the hook won't refetch with the updated options. This could lead to subtle bugs where the UI shows stale data.
Consider either:
- Adding
fetchOptionsto dependencies (may cause infinite loops if the object is recreated each render). - Accepting a stable reference (e.g.,
useMemo'd options from the caller). - Documenting this limitation clearly.
💡 Alternative: Use JSON.stringify for stable comparison
+ const fetchOptionsString = JSON.stringify(fetchOptions);
+
useEffect(() => {
if (!url) {
return;
}
+ const parsedFetchOptions = JSON.parse(fetchOptionsString);
// ... use parsedFetchOptions instead of fetchOptions
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [url, timeout]);
+ }, [url, timeout, fetchOptionsString]);This approach serializes fetchOptions to detect meaningful changes while avoiding reference-equality issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/hooks/useAIFetch.js` around lines 109 - 110, The hook
useAIFetch currently omits fetchOptions from the dependency array (only [url,
timeout]), which can cause stale-closure bugs when fetchOptions (for example
body) changes; update useAIFetch to include a stable representation of
fetchOptions in the deps—either accept and document that callers must pass a
memoized fetchOptions, or add fetchOptions to the dependency list using a stable
comparator such as JSON.stringify(fetchOptions) (or require callers to use
useMemo) so the effect re-runs when options meaningfully change; reference the
useAIFetch effect dependencies and the fetchOptions variable when making this
change.
Comment on lines
+1
to
+5
| import { useState, useEffect } from "react"; | ||
| import "../../index.css"; | ||
| import logo from "../../assets/aossie_logo.webp"; | ||
| import logoPNG from "../../assets/aossie_logo.png"; | ||
| import { normalizeArrayData } from "../../utils/normalizeArrayData"; |
There was a problem hiding this comment.
Import ErrorBoundary and EmptyState before use.
ErrorBoundary (Line 177) and EmptyState (Line 180) are referenced but never imported in this file, which will break rendering.
💡 Proposed fix
import { useState, useEffect } from "react";
import "../../index.css";
import logo from "../../assets/aossie_logo.webp";
import logoPNG from "../../assets/aossie_logo.png";
import { normalizeArrayData } from "../../utils/normalizeArrayData";
+import ErrorBoundary from "../../components/ErrorBoundary";
+import EmptyState from "../../components/EmptyState";Also applies to: 177-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/pages/question/Question.jsx` around lines 1 - 5, The file
references ErrorBoundary and EmptyState but never imports them; add import
statements for ErrorBoundary and EmptyState at the top of the file alongside the
other imports so the JSX at lines where ErrorBoundary and EmptyState are used
can resolve; ensure you import the correct exported names (ErrorBoundary,
EmptyState) from their component modules and place the imports with the existing
React/component imports.
| <ErrorBoundary fallback={<EmptyState message="Quiz failed to load." />}> | ||
| <div className="flex-1 overflow-y-auto scrollbar-hide"> | ||
| {qaPairs && qaPairs.length === 0 ? ( | ||
| <EmptyState message="No quizzes could be parse from the response. Please try again." /> |
There was a problem hiding this comment.
Fix empty-state copy typo.
Line 180 says “could be parse”; it should be “could be parsed”.
💡 Proposed fix
- <EmptyState message="No quizzes could be parse from the response. Please try again." />
+ <EmptyState message="No quizzes could be parsed from the response. 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.
Suggested change
| <EmptyState message="No quizzes could be parse from the response. Please try again." /> | |
| <EmptyState message="No quizzes could be parsed from the response. Please try again." /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/pages/question/Question.jsx` at line 180, Replace the typo in
the EmptyState message prop used in Question.jsx: change the string passed to
the EmptyState component (message="No quizzes could be parse from the response.
Please try again.") to use the correct past participle ("parsed") so it reads
"No quizzes could be parsed from the response. Please try again." Ensure you
update the message prop on the EmptyState JSX element.
| } | ||
|
|
||
| ReactDOM.render(<Question />, document.getElementById("root")); No newline at end of file | ||
| export default Question; No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify how Question is loaded
fd question.html -x sed -n '1,40p' {}
# Verify whether Question self-mounts
fd Question.jsx -x rg -n "ReactDOM\\.render|createRoot|export default Question|function Question" {}
# Compare with an existing mounted page
fd Answer.jsx -x rg -n "ReactDOM\\.render|createRoot" {}Repository: AOSSIE-Org/EduAid
Length of output: 526
🏁 Script executed:
# Search for where Question component is imported and used
rg "Question" --type jsx --type js -B 2 -A 2 | head -100
# Check if there's an initialization or bootstrap script
fd -e js -e jsx | xargs grep -l "Question" | grep -E "(index|init|bootstrap|main)"Repository: AOSSIE-Org/EduAid
Length of output: 128
🏁 Script executed:
# Search for Question imports
rg "from.*Question|import.*Question" -A 3
# Check the main.js file
cat eduaid_desktop/main.js
# Look for any mounting logic in extension directory
find extension -type f \( -name "*.js" -o -name "*.jsx" \) -exec grep -l "render\|createRoot\|mount" {} \;
# Check if question.html is referenced anywhere
rg "question\.html" -lRepository: AOSSIE-Org/EduAid
Length of output: 50376
🏁 Script executed:
# Check if question.html is actually referenced in the app
rg "question\.html" --type html --type js --type jsx
# Check extension directory structure
find extension -name "*.html" -o -name "*.json"
# Look for Question imports in extension directory only
rg "import.*Question\|from.*Question" extension/ --type jsx --type jsRepository: AOSSIE-Org/EduAid
Length of output: 497
🏁 Script executed:
# Search for Question imports/usage
rg "Question" extension/ -l --type-list | head -20
# Check if Question component is imported anywhere
rg "import.*Question|from.*Question" extension/
# Get the full content of Answer.jsx to see the mounting pattern
cat extension/src/pages/answer/Answer.jsx | head -50
# Check Question.jsx to see if it has any initialization
cat extension/src/pages/question/Question.jsx | head -50
# Check if there are any service workers or initialization scripts
find extension -name "*.js" -path "*/src/*" | head -20Repository: AOSSIE-Org/EduAid
Length of output: 4240
Add ReactDOM mounting code to Question.jsx.
The Question component is loaded as a module script by question.html but is never mounted to the DOM. While the HTML file has a #root target element, the component only exports itself without self-bootstrapping. This prevents the page from rendering anything.
Answer.jsx demonstrates the correct pattern with ReactDOM.render(<Answer />, document.getElementById("root"));—Question.jsx needs the same.
💡 Proposed fix
+import ReactDOM from "react-dom";
+
+ReactDOM.render(<Question />, document.getElementById("root"));
export default Question;📝 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
| export default Question; | |
| import ReactDOM from "react-dom"; | |
| ReactDOM.render(<Question />, document.getElementById("root")); | |
| export default Question; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/pages/question/Question.jsx` at line 280, The Question
component is exported but never mounted to the DOM; update Question.jsx to mount
the component after its definition by invoking ReactDOM.render with <Question />
into the page root (document.getElementById("root")) like Answer.jsx does,
ensuring ReactDOM is imported if not already and keeping the existing export
default Question; this will bootstrap the module script and render the component
on question.html.
Comment on lines
+1
to
+5
| import { useState, useEffect } from "react"; | ||
| import { PDFDocument } from 'pdf-lib'; | ||
| import "../../index.css"; | ||
| import logo from "../../assets/aossie_logo.webp"; | ||
| import { normalizeArrayData } from "../../utils/normalizeArrayData"; |
There was a problem hiding this comment.
Import ErrorBoundary and EmptyState before rendering them.
ErrorBoundary and EmptyState are used at Line 225-229 but not imported, which will crash at render time.
💡 Proposed fix
import { useState, useEffect } from "react";
import { PDFDocument } from 'pdf-lib';
import "../../index.css";
import logo from "../../assets/aossie_logo.webp";
import { normalizeArrayData } from "../../utils/normalizeArrayData";
+import ErrorBoundary from "../../components/ErrorBoundary";
+import EmptyState from "../../components/EmptyState";Also applies to: 225-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/pages/question/SidePanel.jsx` around lines 1 - 5, The component
references ErrorBoundary and EmptyState at render (used around the JSX block
where they wrap content, lines ~225-229) but they are not imported; add proper
imports at the top of SidePanel.jsx (for example: import ErrorBoundary from
'...'; import EmptyState from '...') matching your project module paths or
relative locations so the ErrorBoundary and EmptyState symbols are defined
before use in the SidePanel component; ensure the import names match the
exported identifiers and save.
| <ErrorBoundary fallback={<EmptyState message="Quiz failed to load." />}> | ||
| <div className="flex-1 overflow-y-auto scrollbar-hide"> | ||
| {qaPairs && qaPairs.length === 0 ? ( | ||
| <EmptyState message="No quizzes could be parse from the response. Please try again." /> |
There was a problem hiding this comment.
Fix empty-state message grammar.
Line 228 should be “parsed” instead of “parse”.
💡 Proposed fix
- <EmptyState message="No quizzes could be parse from the response. Please try again." />
+ <EmptyState message="No quizzes could be parsed from the response. 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.
Suggested change
| <EmptyState message="No quizzes could be parse from the response. Please try again." /> | |
| <EmptyState message="No quizzes could be parsed from the response. Please try again." /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/pages/question/SidePanel.jsx` at line 228, Update the
grammatically incorrect empty-state message in SidePanel.jsx where the
EmptyState component is rendered; change the message prop value from "No quizzes
could be parse from the response. Please try again." to "No quizzes could be
parsed from the response. Please try again." so the word "parse" is corrected to
"parsed."
Comment on lines
+246
to
+269
| {qaPair.options && ( | ||
| <div className="text-[#FFF4F4] text-[1rem] my-1"> | ||
| {shuffledOptions.map((option, idx) => ( | ||
| <div key={idx} className="flex items-center"> | ||
| <input | ||
| type="radio" | ||
| name={`question${index}`} | ||
| value={option} | ||
| className="mr-2" | ||
| /> | ||
| <span>{option}</span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| )} | ||
| {!qaPair.options && ( | ||
| <div className="my-2"> | ||
| <input | ||
| type="text" | ||
| placeholder="Enter your answer" | ||
| className="bg-[#161E1E] text-white p-2 rounded-lg w-full" | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Handle empty option arrays in rendering fallback.
qaPair.options && treats [] as truthy, so no options are shown and the text-input fallback is skipped. Use an explicit length check.
💡 Proposed fix
- {qaPair.options && (
+ {Array.isArray(qaPair.options) && qaPair.options.length > 0 && (
<div className="text-[`#FFF4F4`] text-[1rem] my-1">
{shuffledOptions.map((option, idx) => (
...
))}
</div>
)}
- {!qaPair.options && (
+ {(!Array.isArray(qaPair.options) || qaPair.options.length === 0) && (
<div className="my-2">
<input
type="text"📝 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
| {qaPair.options && ( | |
| <div className="text-[#FFF4F4] text-[1rem] my-1"> | |
| {shuffledOptions.map((option, idx) => ( | |
| <div key={idx} className="flex items-center"> | |
| <input | |
| type="radio" | |
| name={`question${index}`} | |
| value={option} | |
| className="mr-2" | |
| /> | |
| <span>{option}</span> | |
| </div> | |
| ))} | |
| </div> | |
| )} | |
| {!qaPair.options && ( | |
| <div className="my-2"> | |
| <input | |
| type="text" | |
| placeholder="Enter your answer" | |
| className="bg-[#161E1E] text-white p-2 rounded-lg w-full" | |
| /> | |
| </div> | |
| )} | |
| {Array.isArray(qaPair.options) && qaPair.options.length > 0 && ( | |
| <div className="text-[`#FFF4F4`] text-[1rem] my-1"> | |
| {shuffledOptions.map((option, idx) => ( | |
| <div key={idx} className="flex items-center"> | |
| <input | |
| type="radio" | |
| name={`question${index}`} | |
| value={option} | |
| className="mr-2" | |
| /> | |
| <span>{option}</span> | |
| </div> | |
| ))} | |
| </div> | |
| )} | |
| {(!Array.isArray(qaPair.options) || qaPair.options.length === 0) && ( | |
| <div className="my-2"> | |
| <input | |
| type="text" | |
| placeholder="Enter your answer" | |
| className="bg-[`#161E1E`] text-white p-2 rounded-lg w-full" | |
| /> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/pages/question/SidePanel.jsx` around lines 246 - 269, The
rendering currently uses a truthy check (qaPair.options && ...) which treats an
empty array as truthy so the multiple-choice block renders nothing and the
text-input fallback is skipped; change the condition to explicitly check for a
non-empty array (e.g., Array.isArray(qaPair.options) && qaPair.options.length >
0) for the shuffledOptions mapping and render the text-input fallback when there
are no options; also ensure shuffledOptions is only used/constructed when
qaPair.options is a non-empty array to avoid mapping over undefined.
| } | ||
|
|
||
| ReactDOM.render(<SidePanel />, document.getElementById("root")); | ||
| export default SidePanel; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify how SidePanel is loaded
fd sidePanel.html -x sed -n '1,40p' {}
# Verify whether SidePanel self-mounts
fd SidePanel.jsx -x rg -n "ReactDOM\\.render|createRoot|export default SidePanel|function SidePanel" {}Repository: AOSSIE-Org/EduAid
Length of output: 463
🏁 Script executed:
wc -l extension/src/pages/question/SidePanel.jsxRepository: AOSSIE-Org/EduAid
Length of output: 107
🏁 Script executed:
cat -n extension/src/pages/question/SidePanel.jsx | tail -50Repository: AOSSIE-Org/EduAid
Length of output: 2017
🏁 Script executed:
cat -n extension/src/pages/question/SidePanel.jsx | head -50Repository: AOSSIE-Org/EduAid
Length of output: 1970
SidePanel is exported but not mounted in its HTML entrypoint.
The module loads in sidePanel.html but the component is never rendered into the #root element. Add a mount call at the end of the file.
💡 Proposed fix
+import ReactDOM from "react-dom";
+
+ReactDOM.render(<SidePanel />, document.getElementById("root"));
export default SidePanel;📝 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
| export default SidePanel; | |
| import ReactDOM from "react-dom"; | |
| ReactDOM.render(<SidePanel />, document.getElementById("root")); | |
| export default SidePanel; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/pages/question/SidePanel.jsx` at line 304, The SidePanel
component is exported but never mounted into the DOM; import the React DOM
renderer and render SidePanel into the page's root element
(document.getElementById('root') or '#root') at the end of the file so the
component actually mounts; specifically, add the appropriate React DOM import
(react-dom/client createRoot for React 18 or react-dom render for older
versions) and call createRoot(...).render(<SidePanel />) or
ReactDOM.render(<SidePanel />, root) after the SidePanel export.
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.
Improve Frontend Resilience: Error Boundaries, Timeout Handling, and Safe Data Validation
Resolves Issue #469: Infinite loading loops during backend timeouts.
Resolves Issue #467: Application crashes due to malformed data.
Addressed Issues:
Fixes #469
Fixes #467
Additional Notes:
This PR hardens the frontend React application against backend failures and unexpected data models.
AbortControllerto strictly cap requests at a 60-second timeout, preventing infinite loading UI states..forEach()array iterations with anormalizeArrayDatautility function to prevent rendering crashes when the backend unexpectedly returns non-array objects.AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.