Fix : Software Update Uncaught Promise #10615
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes enhance the error handling and validation logic within the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant checkForUpdate
participant FetchAPI
participant Logger
App->>checkForUpdate: Invoke checkForUpdate()
checkForUpdate->>FetchAPI: Call fetch(update URL)
FetchAPI-->>checkForUpdate: Return response or error
alt Response received
checkForUpdate->>checkForUpdate: Verify res.ok and content-type
alt Valid JSON and version exists
checkForUpdate->>App: Process update metadata
else Invalid content-type or missing version
checkForUpdate->>Logger: Log error message (invalid content/version)
checkForUpdate-->>App: Return early
end
else Fetch error
checkForUpdate->>Logger: Log error from catch block
checkForUpdate-->>App: Return early
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/components/Common/UpdatableApp.tsx (2)
92-94:⚠️ Potential issueFix local storage operation order around page reload.
The
APP_VERSION_KEYupdate might not execute as it's placed afterwindow.location.reload(). The browser might reload before the version is updated.- localStorage.setItem(APP_UPDATED_KEY, "true"); - window.location.reload(); - localStorage.setItem(APP_VERSION_KEY, newVersion); + localStorage.setItem(APP_VERSION_KEY, newVersion); + localStorage.setItem(APP_UPDATED_KEY, "true"); + window.location.reload();
88-88: 🛠️ Refactor suggestionAdd error handling for cache clearing operations.
Cache clearing operations should be wrapped in try-catch to handle potential errors gracefully.
- caches.keys().then((names) => names.forEach((name) => caches.delete(name))); + try { + await caches.keys().then((names) => + Promise.all(names.map((name) => caches.delete(name))) + ); + } catch (error) { + console.error('Failed to clear caches:', error); + }
🧹 Nitpick comments (2)
src/components/Common/UpdatableApp.tsx (2)
52-55: Consider using TypeScript type guards for safer type assertions.Instead of using type assertion, consider using a type guard to ensure type safety at runtime.
- return meta.version as string; + const version = meta.version; + if (typeof version !== 'string') { + console.error('Skipped checking for updates. Version is not a string'); + return; + } + return version;
134-153: Enhance accessibility for update notifications.The popover components should include ARIA attributes for better screen reader support, and the update button should indicate its loading state to assistive technologies.
- <Popover className="rounded-xl bg-alert-600 px-5 py-4 text-white shadow-2xl shadow-alert-900"> + <Popover className="rounded-xl bg-alert-600 px-5 py-4 text-white shadow-2xl shadow-alert-900" + role="alert" + aria-label="Software update available"> {/* ... */} - <Button disabled={isUpdating} onClick={updateApp} variant="alert"> + <Button + disabled={isUpdating} + onClick={updateApp} + variant="alert" + aria-busy={isUpdating} + aria-label={isUpdating ? "Updating software..." : "Update software"}> {isUpdating ? "Updating..." : "Update"} </Button>For the success alert:
- <Popover className="rounded-xl bg-primary-500 px-5 py-4 text-white shadow-2xl shadow-primary-500"> + <Popover className="rounded-xl bg-primary-500 px-5 py-4 text-white shadow-2xl shadow-primary-500" + role="alert" + aria-label="Software update successful">Also applies to: 165-175
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Common/UpdatableApp.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Common/UpdatableApp.tsx (2)
24-28: LGTM! Cache control headers prevent stale version checks.The cache control headers ensure that the browser always fetches the latest build metadata, which is crucial for the update check functionality.
31-50: Robust error handling prevents app unresponsiveness.The enhanced error handling with multiple validation layers (HTTP status, content-type, metadata format) effectively addresses the PR objective of preventing the app from becoming unresponsive during errors.
|
@rithviknishad @khavinshankar Plz review the code i had made the changes but unable to figure out why cypress is failing . It should not fail actually. |
Proposed Changes
Video after Code Changes :
Screencast.from.2025-02-15.03-33-12.webm
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit