[BUG] needs proper error handling in manage form section

[badrivlog]

Description

We have lots of AJAX call in pages/account/manage section and we are calling fetch() function. The problem is here that by default the fetch function doesn't reject the promise if we get back an error status code, in that case, we should reject the promise and throw an actual error and handle the rejected promise.

The downside of not handling a rejected promise in fetch call is that if the request fails for any reason, such as a network error or server-side error, the code does not handle the error gracefully. As a result, the application may not provide appropriate feedback or error handling to the user.

Without proper error handling, it becomes challenging to identify the cause of the issue. Debugging the code and fixing the problem becomes more complex, as there is no specific error message or indication of what went wrong.

To address this issue, it is important to handle rejected promises appropriately. This can involve displaying error messages to the user.

Screenshots

No response

Additional information

want to fix this issue

[github-actions]

To reduce notifications, issues are locked until they are https://github.com/EddieHubCommunity/LinkFree/labels/%F0%9F%8F%81%20status%3A%20ready%20for%20dev and to be assigned. You can learn more in our contributing guide https://github.com/EddieHubCommunity/LinkFree/blob/main/CONTRIBUTING.md

[github-actions]

The issue has been unlocked and is now ready for dev. If you would like to work on this issue, you can comment to have it assigned to you. You can learn more in our contributing guide https://github.com/EddieHubCommunity/LinkFree/blob/main/CONTRIBUTING.md

[eddiejaoude]

Thank you for the suggestion 👍 I think 1 location of code should be updated and then we can discuss further before implementing it to other areas of the code

[Dobariya-Nishant]

I want to work in this issue.

[badrivlog]

My idea is, If the request fails for any reason, such as a network error or server-side error, then reject that promise by throwing an appropriate error message and handle it in the catch block.

One possible solution might be this-
(example code: pages/account/manage/profile.js)

  const handleSubmit = async (e) => {
    e.preventDefault();

    try {
      const res = await fetch(`${BASE_URL}/api/account/manage/profilesf`, {
        method: "PUT",
        headers: {
          "Content-Type": "application/json",
        },
        body: JSON.stringify({ name, bio, tags, layout }),
      });

      if (!res.ok) {
        const update = await res.json();
        throw new Error(update.error || update.message || res.statusText);
      }
      setShowNotification({
        show: true,
        type: "success",
        message: "Profile updated",
        additionalMessage: "Your profile has been updated successfully",
      });
    } catch (err) {
      setShowNotification({
        show: true,
        type: "error",
        message: "Profile update failed",
        additionalMessage: `Please check the fields: ${err.message}`,
      });
    }
}; 

what do you think?

[Dobariya-Nishant]

const handleSubmit = (e) => {
   e.preventDefault();
   fetch(`${BASE_URL}/api/account/manage/profilesf`, {
       method: "PUT",
       headers: {
           "Content-Type": "application/json",
       },
       body: JSON.stringify({ name, bio, tags, layout }),
   })
   .then(async(res) => {
       if (!res.ok) {
           const update = await res.json();
           throw new Error(update.error || update.message || res.statusText);
       }
   })
   .then((data) => {
       setShowNotification({
           show: true,
           type: "success",
           message: "Profile updated",
           additionalMessage: "Your profile has been updated successfully",
       });
   })
   .catch((err) => {
       setShowNotification({
           show: true,
           type: "error",
           message: "Profile update failed",
           additionalMessage: `Please check the fields: ${err.message}`,
       });
   })  
};

@badrivlog When I saw this issue, I thought handling it this way but your try catch and async wait wait is a better approach in my opinion because it's easier to understand.

[badrivlog]

const handleSubmit = (e) => {
   e.preventDefault();
   fetch(`${BASE_URL}/api/account/manage/profilesf`, {
       method: "PUT",
       headers: {
           "Content-Type": "application/json",
       },
       body: JSON.stringify({ name, bio, tags, layout }),
   })
   .then(async(res) => {
       if (!res.ok) {
           const update = await res.json();
           throw new Error(update.error || update.message || res.statusText);
       }
   })
   .then((data) => {
       setShowNotification({
           show: true,
           type: "success",
           message: "Profile updated",
           additionalMessage: "Your profile has been updated successfully",
       });
   })
   .catch((err) => {
       setShowNotification({
           show: true,
           type: "error",
           message: "Profile update failed",
           additionalMessage: `Please check the fields: ${err.message}`,
       });
   })  
};

@badrivlog When I saw this issue, I thought handling it this way but your try catch and async wait wait is a better approach in my opinion because it's easier to understand.

@Dobariya-Nishant I think code you wrote is not correct

[Dobariya-Nishant]

@badrivlog sorry I don't know, it would be great if you correct my code.

[badrivlog]

@badrivlog sorry I don't know, it would be great if you correct my code.

I think you should need to know how promise works. (note: please don't comment issues that are unrelated as this creates extra noise and confusion)

If you need any help- EddieHub Discord might be better
https://discord.gg/44dXjyWB

[eddiejaoude]

Yes we use async/await in our project and we are keen to keep things consistent 👍 thanks for the collaboration everyone, great to see different perspectives 💪

@badrivlog that looks good 👍 I will assign it to you

[badrivlog]

Thank you for the suggestion 👍 I think 1 location of code should be updated and then we can discuss further before implementing it to other areas of the code

Please take a look my draft PR I created.
I have improved the error handling functionality and updated in one location of code.

should I implementing it to other areas of the code?

[eddiejaoude]

Thank you for the suggestion 👍 I think 1 location of code should be updated and then we can discuss further before implementing it to other areas of the code

Please take a look my draft PR I created. I have improved the error handling functionality and updated in one location of code.

should I implementing it to other areas of the code?

I will ask the other maintainers to take a look, I don't see a big benefit, it looks very much the same to me and focused on form validation - so wouldn't work for other http requests 🤔

I think the part we could use is wrapping the http request in a try/catch

[badrivlog]

Yes, we could wrap the http request in a try/catch and handles the error, that might be a solution.
But this approach will produce lots of duplicate code, even of this is kind of repetitive🤔

In opinion it's really a bit cumbersome to having to do all these step all the time. Instead it would be good to actually create ourselves a really nice utility function for api request. And that utility function will wrap up the fetch the error handling, and also the conversion to JSON.

export async function sendRequest(requestConfig) { 
   try { 
     const res = await fetch(requestConfig.url, { 
       method: requestConfig.method ? requestConfig.method : "GET", 
       headers: requestConfig.headers ? requestConfig.headers : {}, 
       body: requestConfig.body ? JSON.stringify(requestConfig.body) : null, 
     }); 
     const data = await res.json(); 
     if (!res.ok) { 
       let errMessage = res.statusText; 
       if (data.message) { 
         if (typeof data.message === "string") errMessage = data.message; 
         else { 
           errMessage = Object.keys(data.message) 
             .map((val) => `${val} is required`) 
             .join(", "); 
         } 
       } 
       if (data.error) { 
         errMessage = data.error; 
       } 
  
       throw new Error(`Request failed! ${errMessage}.`); 
     } 
     return data; 
   } catch (err) { 
     console.error(err); 
     throw err; 
   } 
 }

The sendRequest will accept any kind of api request and when there is an error in this fetch, it will throw a new error and it also immediately converts the response to JSON
The sendRequest function abstract all this functionality into one nice function that we can use all over our project.

[eddiejaoude]

And that utility function will wrap up the fetch the error handling, and also the conversion to JSON.

But part of that code is specific to form validation and not other requests

[badrivlog]

And that utility function will wrap up the fetch the error handling, and also the conversion to JSON.

But part of that code is specific to form validation and not other requests

Hey, thank you for correcting me💪
I have pushed new changes to my PR, please take a look 🙏

What's next?

[badrivlog]

Hi @eddiejaoude ,

I hope you're doing well. I've submitted a pull request for Handling rejected promise and I would really appreciate your feedback. Your expertise would be invaluable in ensuring the quality and functionality of the code changes.

Please take a moment to review the pull request at #8283 and share your thoughts. Your insights and suggestions will help me to improve the codebase and deliver a better product.

Thank you in advance for your time and effort!

[eddiejaoude]

Thanks @badrivlog sure I will take a look and ask the maintainers to also as this is a challenging topic 👍