Skip to content
This repository was archived by the owner on Apr 4, 2022. It is now read-only.

Failed to fetch notification #347

Closed
MareenG opened this issue Mar 30, 2021 · 20 comments · Fixed by #992
Closed

Failed to fetch notification #347

MareenG opened this issue Mar 30, 2021 · 20 comments · Fixed by #992
Assignees
Labels
app:Explorer Explorer App Bug Something isn't working Protofire task to the protofire team

Comments

@MareenG
Copy link

MareenG commented Mar 30, 2021

I got the following message below the order details
Bildschirmfoto 2021-03-30 um 10 31 28

It appears and disappears. After refreshing the website only the error message was shown

@MareenG MareenG added Bug Something isn't working app:Explorer Explorer App labels Mar 30, 2021
@MareenG
Copy link
Author

MareenG commented Mar 30, 2021

I also got several failed to fetch notifications for Cowswap today( between 10 and 11am). Now they seem to be resolved

@alfetopito
Copy link
Contributor

That is a generic error message that we display when we fail to fetch the order details from the backend.
You see it appearing and disappearing because while an order is not expired, the UI will keep updating the order details in the background to check whether it has been executed without the need to refresh the page.
And since it was failing, the error message was displayed.

There are already pending issues to better handle the error message.

Regarding the issue, might have been an sporadic network or backend failure. Can't tell at this point.

@anxolin
Copy link
Contributor

anxolin commented Mar 30, 2021

There might be also a technical reason to see this non informative message.

details: This is also likely related to one issue @vkgnosis discovered today. When the ther backend returns an Error, it's sometimes not returning the CORS headers, this makes the actual request to fail, so even if it's a controlled and nice typed error, the FE will see it as a failed fetch request.

@anxolin
Copy link
Contributor

anxolin commented Apr 14, 2021

Linking this to this related issue gnosis/cowswap#339

@elena-zh
Copy link

elena-zh commented Oct 4, 2021

In order to reproduce this issue:

  1. Create an order
  2. While the order is in the 'Open' status, open the order details in the Explorer
  3. Switch off the internet connection
    image

@alfetopito alfetopito added the Protofire task to the protofire team label Oct 4, 2021
@alfetopito
Copy link
Contributor

@alongoni @fapesteguia @maria-vslvn this would be a nice issue to tackle next.

@alongoni alongoni self-assigned this Oct 4, 2021
@alongoni
Copy link
Contributor

alongoni commented Oct 4, 2021

Perhaps using toast notifications would be a good idea?

UI_Explorer v1  - order detail page _ failed to fetch order notification

Like this one https://www.npmjs.com/package/react-toastify
cc @alfetopito

@alfetopito
Copy link
Contributor

Yes, that's one idea we discussed earlier.

What do you think regarding that kind of UX @biocom @anxolin

@fairlighteth
Copy link
Contributor

I see different objectives

  1. User is offline > can't fetch order
  2. A specific error occurred fetching an order

For both objectives I regard a toast notification inappropriate for the purpose it intends to serve. A toast notification IMO should be shown after an explicit user initiate action (e.g. you submitted an order -> getting feedback from a toast notification or an order gets settled, etc.).

Instead

  1. I think we should show a 'You're offline' banner on top.
  2. Show an error banner more close to the table or somewhere below the page header.

The offline banner (point 1 above) could also be shown just like point 2.

@alfetopito
Copy link
Contributor

I'm wondering now about this.

The UI is still functional, you can still see everything about your order.
There's only no background update. Or one update failed. A page refresh "should" be enough to fix, if needed.

What if we don't say anything at all for this type of error?

Of if we do, something like point 2 with a subtle error message like "background update failed, refresh to see new stuff"

@alongoni
Copy link
Contributor

alongoni commented Oct 5, 2021

I'm wondering now about this.

The UI is still functional, you can still see everything about your order. There's only no background update. Or one update failed. A page refresh "should" be enough to fix, if needed.

What if we don't say anything at all for this type of error?

Of if we do, something like point 2 with a subtle error message like "background update failed, refresh to see new stuff"

hey @alfetopito, something like this?

UI_Explorer v1  - order detail page _ failed to fetch order notification

@alfetopito
Copy link
Contributor

Yes, that is more subtle.

@biocom good with you?

@fairlighteth
Copy link
Contributor

Some comments:

  • I would place it below (or above) the Order Details title and have it occupy 100% width.

Screen Shot 2021-10-07 at 09 38 28

  • Would use different color backgrounds depending on the severity of the alert (e.g. warning = yellow/orange & error = red)

@alongoni
Copy link
Contributor

alongoni commented Oct 7, 2021

Design updated:

Warning:
UI_Explorer v1  - order detail page _ failed to fetch order notification

Error:
UI_Explorer v1  - order detail page _ failed to fetch order notification-1

@fairlighteth
Copy link
Contributor

Looks good @alongoni Small nitpick: I would left align the text, to match the other left aligned content on the rest of the page.

@elena-zh
Copy link

elena-zh commented Oct 8, 2021

What about adding the link to the 'try again' what will refresh the page?

@alongoni
Copy link
Contributor

Design updated:

  • text align to left.
  • add "try again" link.
    UI_Explorer v1  - order detail page _ failed to fetch order notification
    UI_Explorer v1  - order detail page _ failed to fetch order notification-1

@fapesteguia
Copy link

@maria-vslvn Can you handle this issue?

@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you think it shouldn't be closed, speak now or forever hold your peace.

@stale stale bot added the wontfix Stale issue label Mar 2, 2022
@elena-zh
Copy link

elena-zh commented Mar 2, 2022

The issue is a WIP in #992 pr

@stale stale bot removed the wontfix Stale issue label Mar 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
app:Explorer Explorer App Bug Something isn't working Protofire task to the protofire team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants