Enhance IP data handling in ShikshalokamChat and ShikshalokamVoiceChat components - #207
Conversation
…t components by introducing ipFetched state management
WalkthroughThe changes introduce an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.0)src/pages/ShikshalokamVoiceChat/voice-chat.js[] 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/ShikshalokamVoiceChat/voice-chat.js(3 hunks)src/pages/shikshalokamChat.js(4 hunks)src/store/slices/userData/state.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/store/slices/userData/state.js (2)
src/pages/ShikshalokamVoiceChat/voice-chat.js (2)
ipZipCode(173-173)ipFetched(174-174)src/pages/shikshalokamChat.js (1)
ipFetched(40-40)
src/pages/ShikshalokamVoiceChat/voice-chat.js (1)
src/pages/shikshalokamChat.js (2)
ipFetched(40-40)useUserStorage(18-18)
src/pages/shikshalokamChat.js (7)
src/pages/ShikshalokamVoiceChat/voice-chat.js (7)
useUserStorage(180-180)useChatStorage(177-177)useChatStorage(181-181)storageFlow(167-167)ipFetched(174-174)sessionId(158-158)isLoading(94-94)src/hooks/useStorage.js (4)
useUserStorage(24-26)useUserStorage(24-26)useChatStorage(20-22)useChatStorage(20-22)src/pages/UnifiedChat/UnifiedChat.jsx (5)
useUserStorage(18-18)useChatStorage(19-19)storageFlow(22-22)sessionId(23-23)isLoading(15-15)src/pages/Login/commonPage.jsx (2)
storageFlow(31-31)setFlow(32-32)src/pages/UnifiedChat/StoryActionsModule.js (6)
storageFlow(32-32)storageFlow(284-284)sessionId(31-31)sessionId(283-283)isLoading(281-281)isLoading(563-563)src/pages/ShikshalokamVoiceChat/shikshaChatSidebar.js (1)
storageFlow(18-18)src/services/api.service.js (2)
getIpLocation(63-66)getIpLocation(63-66)
🔇 Additional comments (7)
src/store/slices/userData/state.js (1)
12-12: LGTM! Clean state addition.The new
ipFetchedflag and setter follow the existing patterns in this state file and provide a clear way to track IP data fetch completion.Also applies to: 20-21
src/pages/ShikshalokamVoiceChat/voice-chat.js (3)
210-210: Good refactor of dependency array.Replacing individual IP field dependencies with
ipFetchedcorrectly reflects that authentication should only occur once after IP data is available, rather than re-authenticating when individual IP fields change.
191-193: Verify WebSocket connection sequencing and authentication flow.The early return when
!ipFetchedprevents sending authentication, which could leave the WebSocket connected but unauthenticated. Confirm that the WebSocket connection is only attempted afteripFetchedbecomestrue, or verify that retry/deferred authentication logic handles this scenario appropriately.
204-209: Verify backend API compatibility with address object structure.The address field uses an object format with
ipCity,ipState, andipZipCodekeys. Confirm the backend WebSocket authentication endpoint supports this structure and that these fields are reliably populated before deployment.src/pages/shikshalokamChat.js (3)
10-10: Good cleanup of unused import.Removing
useSiteStoragefrom the imports appears to be a valid cleanup since it's not used in this file.
34-34: LGTM! Correct state access.The new
setIpFetchedandipFetchedaccessors are correctly retrieved from state following existing patterns.Also applies to: 40-40
167-167: Loader condition depends on correct ipFetched state.The loader visibility now depends on both
isLoadingandipFetchedstates. Ensure the critical issue in lines 127-154 is resolved so thatipFetchedaccurately reflects initialization success, not just completion.
| try { | ||
| // if (accessToken) return | ||
|
|
||
| if (!sessionId) { | ||
| // clearFromStorage(false, ["local_route"]) | ||
| setIsLoading(true) | ||
| // setHasAcceptedTnc("ONGOING") | ||
| setIsNewChatOpen(true) | ||
|
|
||
| const locationData = await getIpLocation() | ||
| if (locationData && locationData?.location) { | ||
| setIpState(locationData?.location?.regionName) | ||
| setIpCity(locationData?.location?.city) | ||
| setIpCountry(locationData?.location?.country) | ||
| setIpZipCode(locationData?.location?.zip) | ||
| } | ||
| setFlow(type) | ||
| getUserFingerPrint() | ||
| await setFinalLanguage() | ||
|
|
||
| setIsLoading(false) | ||
| } | ||
| setFlow(type) | ||
| getUserFingerPrint() | ||
| await setFinalLanguage() | ||
|
|
||
| setIsLoading(false) | ||
| // else if (storageFlow && !accessToken){ | ||
| // window.location.reload(); | ||
| // } | ||
| } finally { | ||
| setIpFetched(true) | ||
| } |
There was a problem hiding this comment.
Fix error handling: ipFetched set to true even on failure.
The finally block sets ipFetched = true regardless of whether the IP location fetch succeeds or fails. This means:
- If
getIpLocation()fails, the loader will still disappear (line 167) - Failed initialization will be masked from users
- The WebSocket may attempt to authenticate with undefined IP values
Consider setting ipFetched = true only after successful completion, or add a separate error state to handle failures properly.
Apply this diff to fix the error handling:
const runSetup = async () => {
- try {
- // if (accessToken) return
-
- if (!sessionId) {
- // clearFromStorage(false, ["local_route"])
- setIsLoading(true)
- // setHasAcceptedTnc("ONGOING")
- setIsNewChatOpen(true)
-
- const locationData = await getIpLocation()
- if (locationData && locationData?.location) {
- setIpState(locationData?.location?.regionName)
- setIpCity(locationData?.location?.city)
- setIpCountry(locationData?.location?.country)
- setIpZipCode(locationData?.location?.zip)
- }
- setFlow(type)
- getUserFingerPrint()
- await setFinalLanguage()
-
- setIsLoading(false)
+ if (!sessionId) {
+ try {
+ setIsLoading(true)
+ setIsNewChatOpen(true)
+
+ const locationData = await getIpLocation()
+ if (locationData && locationData?.location) {
+ setIpState(locationData?.location?.regionName)
+ setIpCity(locationData?.location?.city)
+ setIpCountry(locationData?.location?.country)
+ setIpZipCode(locationData?.location?.zip)
+ }
+ setFlow(type)
+ getUserFingerPrint()
+ await setFinalLanguage()
+
+ // Only set ipFetched to true on successful completion
+ setIpFetched(true)
+ } catch (error) {
+ console.error("Error during IP location fetch:", error)
+ // Consider showing error to user or setting error state
+ } finally {
+ setIsLoading(false)
}
- // else if (storageFlow && !accessToken){
- // window.location.reload();
- // }
- } finally {
- setIpFetched(true)
}
}🤖 Prompt for AI Agents
In src/pages/shikshalokamChat.js around lines 127 to 154, the finally block
unconditionally sets ipFetched=true which hides the loader and allows downstream
flows to proceed even if getIpLocation() fails; instead move setIpFetched(true)
into the successful path (after IP values are set) and add a catch block that
sets an explicit error state (e.g., setIpError or setIpFetched(false)) and
ensures setIsLoading(false) so failures are visible; also guard later
WebSocket/auth logic to not use undefined IP values (or set sensible defaults)
when ipFetch failed.
2 participants
Enhance IP data handling in ShikshalokamChat and ShikshalokamVoiceChat components by introducing ipFetched state management
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.