Implemented Error Handling for API Calls in API depended components

[pbelokon]

Closes #86

Components affected are:

• BreadcrumbNavigation
• HierarchySwitcher
• ListOfRegions
• RegionMap

Changes:

I have wrapped all functions that do API calls in try-catch removed some unnecessary checks . Also, added a useState for errors "user-friendly notifications"(just used what you had and changed them a little) + Components have fallback states for when API calls fail.

In ListOfRegions, I think that having to check if (newRegions.length > 0) { setRegions(newRegions); } did not changed anything without it. React probably accounts for empty arrays.

---

Let me know if I missed anything

Summary by CodeRabbit

• New Features

  • Enhanced error handling across various components to improve user feedback during data fetching failures.

• Bug Fixes

  • Implemented error states in components to display error messages when data fetching encounters issues.

• Refactor

  • Updated useEffect and event handler functions with try...catch blocks for robust error management.

• Style

  • Adjusted UI to conditionally display error messages within affected components.

[coderabbitai]

Walkthrough

The code changes across multiple frontend components introduce a standardized error handling mechanism for API calls. The updates include the addition of error states using the useState hook, wrapping API interactions within try...catch blocks, and implementing user-friendly error notifications. These modifications aim to enhance the application's reliability and user experience by ensuring graceful handling and reporting of errors during API interactions.

Changes

File Path	Change Summary
.../BreadcrumbNavigation.jsx	Added error state and error handling in API calls.
.../HierarchySwitcher.jsx	Introduced error state and improved error handling in API interactions.
.../ListOfRegions.jsx	Implemented error state management and error handling during data fetching.
.../RegionMap.jsx	Added error state handling for fetching region geometry.

Assessment against linked issues

Objective	Addressed	Explanation
Review all existing API calls in the frontend codebase. (#86)	✅
Design a standardized error handling approach applicable to all API interactions. (#86)	✅
Ensure user notification and graceful application handling for API failures. (#86)	✅
Wrap all frontend API calls with try-catch blocks or use promise chaining with .catch() for error handling. (#86)	✅
Implement logging for developers and user-friendly notifications for errors. (#86)	❓	It's unclear if logging for developers has been implemented as there's no mention of logging mechanisms.
Define fallback states or behaviors for components when API calls fail. (#86)	❓	The summary mentions error states but does not specify fallback behaviors.
Consider using a global error handling context or a higher-order component for repetitive error handling logic. (#86)	❌	There is no mention of a global context or higher-order component for error handling, suggesting this has not been addressed.
Address the issue progressively to enhance application stability and reliability. (#86)	✅

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 X ?

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• You can reply to a review comment made by CodeRabbit.
• You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
• You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 4

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between 359ffa4 and dc8d755.

Files selected for processing (4)

• frontend/src/components/BreadcrumbNavigation.js (1 hunks)
• frontend/src/components/HierarchySwitcher.js (2 hunks)
• frontend/src/components/ListOfRegions.js (1 hunks)
• frontend/src/components/RegionMap.js (1 hunks)

Additional comments: 13

frontend/src/components/BreadcrumbNavigation.js (3)

• 12-29: The error handling in the useEffect hook is implemented correctly, and the error state is being reset when a new attempt to fetch ancestors is made.

• 32-53: The handleBreadcrumbClick function has been updated to handle errors correctly, and the error state is being reset when a new attempt to fetch region details is made.

• 55-57: The error message is displayed conditionally based on the error state, which is a good practice for user feedback.

frontend/src/components/HierarchySwitcher.js (1)

• 13-24: The useEffect hook implementation for fetching hierarchies and setting the error state is correct and follows best practices for async operations within the hook.

frontend/src/components/ListOfRegions.js (4)

• 12-12: The addition of the error state for error handling is a good practice for improving user experience by displaying error messages when fetching regions fails.

• 14-30: The error handling in the fetchRegions function with a try-catch block is a robust way to manage API call failures and to set the error state accordingly.

• 33-35: The useEffect hook has been correctly updated to call fetchRegions with the selectedRegion.id and selectedRegion.hasSubregions as parameters, ensuring that the regions are fetched whenever the selectedRegion or selectedHierarchy changes.

• 51-51: The conditional rendering of the error message based on the error state is a good practice for user feedback. It ensures that users are informed when there is an issue fetching the regions.

frontend/src/components/RegionMap.js (5)

• 1-1: The addition of useState to the import from 'react' is appropriate for managing the new error state within the component.

• 10-10: The introduction of an error state using useState is a good practice for handling errors in React components.

• 12-26: The use of a try-catch block within fetchSelectedRegionGeometry for error handling and setting the error state is a good practice for robustness and user feedback.

• 29-82: The modification of the useEffect hook to handle the error state and log errors when fetching region geometry is a good practice for error handling in React components.

• 84-89: The conditional rendering of the error message in the component's return statement is a good practice for user experience, as it provides feedback to the user when an error occurs.

[pbelokon]

I think my code editor applied formatting sorry ! is there a script for formatting I can use ?

[OhmSpectator]

is there a script for formatting I can use ?

@pbelokon, unfortunately, not at the moment. However, I will make sure to add it later. For now, let me tell you that I use WebStorm schema for JavaScript and 4 spaces for indentation. If I come across a way to share it in a standard format, I will definitely let you know. In the meantime, it would be great if you could continue to use 4 spaces for indentation.

In any case, thanks for your PR! I'll review it in the next few days =)

[pbelokon]

spaces for indenta

Sounds great, thank you! I will wait for updates.

[OhmSpectator]

In addition to the inline comments I would ask to avoid the unnecessary whitespace changes. Sorry for not having a formatting script now. I would suggest just sticking to 4-space indentation.

In any case, thanks for your time!

[OhmSpectator]

Hey @pbelokon, I wanted to check with you if you're still interested in working on this PR? Can you please confirm?

[pbelokon]

Hey @pbelokon, I wanted to check with you if you're still interested in working on this PR? Can you please confirm?

Yes I am, sorry I had a lot of work so could not work on it earlier.

[OhmSpectator]

@pbelokon no problem! Thanks for the confirmation!

[OhmSpectator]

@pbelokon, I've updated the main branch to conform with Airbnb's Code Style. If you want to proceed with the PR, run npm run lint and npm run lint:fix to ensure adherence to the style guide.

[coderabbitai]

Review Status

Actionable comments generated: 4

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between dc8d755 and 6ae4c34.

Files selected for processing (2)

• frontend/src/components/HierarchySwitcher.js (2 hunks)
• frontend/src/components/RegionMap.js (1 hunks)

Additional comments: 4

frontend/src/components/HierarchySwitcher.js (4)

• 7-10: The addition of the error state using the useState hook aligns with the PR objectives to enhance error handling mechanisms.

• 13-22: The useEffect hook correctly implements error handling for API calls by using a try-catch block and setting the error state upon catching an error.

• 36-51: The handleHierarchyChange function is updated to handle errors by setting the error state, which is consistent with the PR's goal of standardized error handling.

• 13-24: The summary mentions updates to the useEffect cleanup function in the RegionMap component, which is not part of the provided code. Ensure that the cleanup logic is correctly implemented in the RegionMap component as per the PR objectives.

[OhmSpectator]

@pbelokon, could you please rebase it on top of the main branch?

[pbelokon]

@pbelokon, could you please rebase it on top of the main branch?

I think I did it can you verify ? , sorry not great with rebasing

[OhmSpectator]

I think I did it can you verify ? , sorry not great with rebasing

@pbelokon, just checking in - I'm not sure about your level of experience with Git, so don't take offence if this is stuff you already know! When working with a fork, there are a few ways to ensure your branch is correctly rebased onto the updated main branch of the original repo. Here’s how you can do it:

Sync with the Original Repo:

If you haven't set up the original repo as a remote yet, here's how to do it:

git remote add upstream git@github.com:OhmSpectator/track-your-regions.git

Grab the Latest Updates:

Pull the latest changes from the original repo:

git fetch upstream

Or, use the GitHub UI: There's a "Sync fork" button on your fork's GitHub page that does the same job as the previous 2 actions.

Switch to Your Branch:

Make sure you're on your feature branch:

git checkout issue-86

Start Rebasing:

Rebase your changes onto the main branch of the original repo:

git rebase upstream/main

And then the hard part is starting.

Tackling Merge Conflicts:

Encountering conflicts? They'll look like this in your code: <<<<<<<, =======, >>>>>>>. Edit the files to fix these, and then:

git add [file-name]
git rebase --continue

Repeat until all conflicts are resolved.

Pushing Back:

After fixing everything, push your changes (force push is needed here):

git push origin issue-86 --force

Tip: If your changes are small and the difference in the base branch is huge, sometimes it's easier to start fresh and manually redo your changes on a new branch based on the latest main.

Also, here is a link to an article that provides some details about the merge conflict resolving. https://www.atlassian.com/git/tutorials/using-branches/merge-conflicts

Feel free to give this a go and let me know if you run into any snags!

[pbelokon]

@OhmSpectator ok I think I did it thank you for your help

[coderabbitai]

Review Status

Actionable comments generated: 5

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between 893b79d and 8e08747.

Files selected for processing (4)

• frontend/src/components/BreadcrumbNavigation.js (1 hunks)
• frontend/src/components/HierarchySwitcher.js (1 hunks)
• frontend/src/components/ListOfRegions.js (1 hunks)
• frontend/src/components/RegionMap.js (1 hunks)

Additional comments: 7

frontend/src/components/BreadcrumbNavigation.js (5)

• 10-10: The introduction of a local error state in BreadcrumbNavigation to handle API call failures is consistent with the PR's objective to enhance error handling.

• 12-29: The useEffect hook correctly implements a try-catch block for API calls, ensuring that errors are caught and handled appropriately.

• 32-53: The handleBreadcrumbClick function now includes error handling, which is in line with the PR's objective to enhance error handling for API calls.

• 57-57: The conditional rendering of error messages based on the error state is a user-friendly way to notify users of errors, which aligns with the PR's objectives.

• 63-63: As previously discussed, the unnecessary third parameter item.hasSubregions in the onClick handler will be addressed in a separate PR.

frontend/src/components/ListOfRegions.js (2)

• 14-30: The implementation of error handling in the fetchRegions function is well done. It correctly catches exceptions, logs the error, and sets a user-friendly error message to the state.

• 51-51: The conditional rendering of the error message is a good practice, ensuring that users are informed when an error occurs during the API call.

[OhmSpectator]

@pbelokon, there is another problem now. The files you've edited were renamed on our end after your changes, which means we need to reapply your edits to the new file names. Plus, there are a few lint errors to sort out (see https://github.com/OhmSpectator/track-your-regions/actions/runs/7145970039/job/19478309068?pr=88).

This kind of situation is actually quite common in open source projects, and in any project where multiple people are working on the codebase simultaneously. Especially in the early stages of development, things like file renaming and enforcing coding styles can lead to such conflicts. In our case, the renaming and conflicts arose as I've been implementing some coding style standards that weren't in place before. It's a natural part of evolving and refining our project.

So, here's my plan: I’ll handle these issues for you. I'll take your branch, resolve the conflicts due to the file renaming, and fix the lint errors. After that, I'll create a new PR to integrate these changes. And don't worry, your original authorship on the commits will be completely preserved in the Git history. Your contribution will be clearly credited.

[pbelokon]

PR to integrate these changes. And don't worry, your original authorship on t

sure, thank you for the explanation and help

[coderabbitai]

Review Status

Actionable comments generated: 4

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between 935f695 and 692ddca.

Files selected for processing (4)

• frontend/src/components/BreadcrumbNavigation.jsx (1 hunks)
• frontend/src/components/HierarchySwitcher.jsx (2 hunks)
• frontend/src/components/ListOfRegions.jsx (2 hunks)
• frontend/src/components/RegionMap.jsx (2 hunks)

Additional comments: 20

frontend/src/components/BreadcrumbNavigation.jsx (5)

• 10-10: The addition of an error state using useState is consistent with the PR's objective to handle errors during data fetching.

• 14-26: The implementation of a try-catch block within the useEffect hook to handle API call errors and set the error state accordingly aligns with the PR's goal of enhancing error handling.

• 32-51: The refactoring of the handleBreadcrumbClick function to include error handling with a try-catch block and setting the error state is in line with the PR's objectives.

• 56-70: The conditional rendering of the error message based on the error state is a good user experience practice, making it clear when an error has occurred.

• 29-29: Verify that the useEffect hook does not lead to setting state on an unmounted component, which can cause memory leaks or errors in React. This can be mitigated by using a cleanup function or a ref to track the mounted state of the component.

#!/bin/bash
# Check for patterns where state is set after an asynchronous operation without checking if the component is still mounted.
ast-grep --lang javascript --pattern $'useEffect(() => {
  $$$
  $_.then($_ => {
    $$$
    setState($_)
  })
  $$$
})'

frontend/src/components/HierarchySwitcher.jsx (4)

• 13-21: The implementation of the try-catch block in the useEffect hook for fetching hierarchies is correct and aligns with the PR's objective to improve error handling. Logging the error and setting an error state when an exception occurs will provide better user feedback.

• 35-50: The addition of a try-catch block in the handleHierarchyChange function is a good practice for error handling. It ensures that any errors during the hierarchy update process are caught, logged, and communicated to the user through the error state.

• 55-55: The conditional rendering of the error message based on the error state is a good practice. It ensures that the error message is only displayed when there is an error, which is a user-friendly way to handle errors.

• 46-46: Resetting the error state to null after a successful hierarchy change is a good practice. It ensures that previous errors do not persist and affect the user experience after the issue has been resolved.

frontend/src/components/ListOfRegions.jsx (4)

• 8-33: The implementation of error handling in the fetchRegions function with a try-catch block and the addition of an error state with useState are in line with the PR objectives to enhance error handling. The logic to reset the error state when a new fetch operation is successful is also present, which is good practice for user experience.

• 33-33: The useEffect hook is correctly set up with dependencies on selectedRegion and selectedHierarchy, ensuring that the fetchRegions function is called when these values change. This is consistent with React best practices for hooks.

• 51-57: The conditional rendering of the error message in red when there is an error aligns with the PR's objective to provide user-friendly error notifications. This is a good user experience practice.

• 8-33: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [62-62]

The ListOfRegions component is correctly exported, which is consistent with the PR's objective to update and export the component.

frontend/src/components/RegionMap.jsx (7)

• 1-1: The addition of useState to the import statement from React is in line with the PR objectives to manage error states.

• 12-12: Initialization of the error state using useState is consistent with the PR objectives to handle errors from API calls.

• 14-45: The implementation of try-catch in fetchSelectedRegionGeometry for error handling during API calls aligns with the PR objectives.

• 43-44: Logging the error to the console and setting a user-friendly error message with setError is a good practice for both debugging and user experience.

• 49-49: The cleanup function in the useEffect hook that removes the map instance is consistent with the user's preference for idempotency in managing the map instance.

• 49-49: The return statement correctly renders the map container, which is standard practice in React components.

• 49-49: The MapComponent function is correctly exported as a default export, aligning with the PR objectives.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between 935f695 and b985569.

Files selected for processing (4)

• frontend/src/components/BreadcrumbNavigation.jsx (1 hunks)
• frontend/src/components/HierarchySwitcher.jsx (2 hunks)
• frontend/src/components/ListOfRegions.jsx (2 hunks)
• frontend/src/components/RegionMap.jsx (3 hunks)

Files skipped from review as they are similar to previous changes (4)

• frontend/src/components/BreadcrumbNavigation.jsx
• frontend/src/components/HierarchySwitcher.jsx
• frontend/src/components/ListOfRegions.jsx
• frontend/src/components/RegionMap.jsx

[OhmSpectator]

I've rebased the branch on top of main and resolved the conflicts. I will do the proper review in the next few days.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between 935f695 and 3c74fdf.

Files selected for processing (4)

• frontend/src/components/BreadcrumbNavigation.jsx (1 hunks)
• frontend/src/components/HierarchySwitcher.jsx (2 hunks)
• frontend/src/components/ListOfRegions.jsx (2 hunks)
• frontend/src/components/RegionMap.jsx (3 hunks)

Files skipped from review as they are similar to previous changes (4)

• frontend/src/components/BreadcrumbNavigation.jsx
• frontend/src/components/HierarchySwitcher.jsx
• frontend/src/components/ListOfRegions.jsx
• frontend/src/components/RegionMap.jsx

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between 935f695 and b0e3182.

Files selected for processing (4)

• frontend/src/components/BreadcrumbNavigation.jsx (1 hunks)
• frontend/src/components/HierarchySwitcher.jsx (2 hunks)
• frontend/src/components/ListOfRegions.jsx (2 hunks)
• frontend/src/components/RegionMap.jsx (3 hunks)

Files skipped from review as they are similar to previous changes (4)

• frontend/src/components/BreadcrumbNavigation.jsx
• frontend/src/components/HierarchySwitcher.jsx
• frontend/src/components/ListOfRegions.jsx
• frontend/src/components/RegionMap.jsx

[OhmSpectator]

I fixed the PR and created a follow-up issue (#142). Looks good to me now.