Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alert user on none-existing file when 404 from server #172

Closed
stijnvanhoey opened this issue Oct 10, 2022 · 17 comments
Closed

Alert user on none-existing file when 404 from server #172

stijnvanhoey opened this issue Oct 10, 2022 · 17 comments
Labels
enhancement New feature or request
Milestone

Comments

@stijnvanhoey
Copy link
Collaborator

stijnvanhoey commented Oct 10, 2022

When files are not available server-side there is a 404 in the app, but this information is not passed to the frontend/user. Also a 'future'-file request should return the same error.

@peterdesmet
Copy link
Member

I think this would be a very useful feature.

@peterdesmet peterdesmet added this to the Aloft milestone Apr 11, 2023
@niconoe
Copy link
Collaborator

niconoe commented Apr 21, 2023

@peterdesmet / @stijnvanhoey: do you need help for this? How would you like the error to be shown? Red message below the intro text?

@peterdesmet
Copy link
Member

peterdesmet commented Apr 21, 2023

Thanks @niconoe, help would be appreciated. I would would show a message in the top chart, as that's where people's eyes will be drawn to:

CROW

  • Only show in top chart (bottom chart can be ignored)
  • Show in the center of the chart
  • Message should make use of translation
  • When 3 days are selected, only show message if none of the days have data. The moment one day has data, show as normal:

CROW_3days

@peterdesmet
Copy link
Member

Note: the implementation at #193 will show No data found if no data is pushed to the chart (for whatever reason). It has the advantage that it is simple to implement.

It isn't great for debugging (causes might be 404, data not being processed correctly like for NL radars a while ago, etc.), but it at least informs the user. The most common use case is that there is no source data.

@niconoe
Copy link
Collaborator

niconoe commented Apr 24, 2023

Note: in addition to this, we can easily add another message (displayed globally above all charts - that's a global issue that affect all charts, after all) when a 404 error is encountered, that might help for debugging. In most cases the two messages would appear at the same time, but not always (example: when a chart is partially filled, but one day of data is missing).

@peterdesmet
Copy link
Member

From a UI point of view, I would prefer to include this in the main message in the chart.

No data

And in case of 404:

No data found (404)

@peterdesmet
Copy link
Member

Let me rephrase, I think the following would make sense. I don't know what is easy to implement though.

  1. Chart is empty be default when it's loading (current behaviour)

  2. In case one of the requested files returns a 404 and no data is pushed to the chart, show No data found (404) on chart.

Screenshot 2023-04-24 at 17 32 59

No message is shown here, because other dates (with data) push data to chart:

Screenshot 2023-04-24 at 17 32 07

  1. In case no data is pushed to the chart after 4 seconds (and cause is not 404), show Cannot process data.

Screenshot 2023-04-24 at 17 33 57

I'm open to alternative approaches, but I would prefer to show information in a single place only, preferably on the chart. @CeciliaNilsson709 @baptischmi any preference/suggestions?

@niconoe
Copy link
Collaborator

niconoe commented Apr 25, 2023

@peterdesmet: I initially tried something like that, but that lead to circumvented code and ambiguous messages for a series of reason (multiple components with different concerns, and edge cases, I can elaborate if necessary).

If your request for 404 errors is solely for debugging purposes, would that work to not show them directly to the end-users (keeping it like it is in the PR now), but log the 404 in the JS console?

@peterdesmet
Copy link
Member

but log the 404 in the JS console?

Yes, an informative message in the JS for every file that returns a 404 would be good.

@CeciliaNilsson709
Copy link

I think for the end user an important distinction would be between no data (for whatever reason) and data but no birds. Is that possible, for example in Peters second case above?

@peterdesmet
Copy link
Member

no data (for whatever reason) and data but no birds

The difference would be a message saying No data found (for whatever reason) and an empty chart.

@CeciliaNilsson709
Copy link

So not like in your second example then?

@niconoe
Copy link
Collaborator

niconoe commented Apr 27, 2023

but log the 404 in the JS console?

Yes, an informative message in the JS for every file that returns a 404 would be good.

Hmmm I wanted to implement that, but realize that if it is only for debugging purposes, it's already provided by the browser 🥇

Screenshot 2023-04-27 at 11 20 45

@peterdesmet
Copy link
Member

peterdesmet commented Apr 27, 2023

@CeciliaNilsson709 you are right, for charts that have some data (April 24), it would not be possible for users to know whether April 25 and 26 have no bird data or if there was no data found:

234045793-fbf90208-7c6a-4b5d-9aa4-7877150a0805

@niconoe do you think it possible to show the No data found message per day (pinned at noon) in the chart? Or a grayed out area?

@niconoe
Copy link
Collaborator

niconoe commented Apr 27, 2023

Well everything is possible but I think we should take care of keeping the complexity manageable given the nasty edge cases (for example, the 1:1 correlation between days and files only works only in UTC mode)

@peterdesmet
Copy link
Member

Yeah, thinking about it a bit more, then only way I see to correctly differentiate between no bird data and no data found is by graying out the area, like so (this is actual data):

Screenshot 2023-04-27 at 12 06 06 copy

@niconoe I'm not sure how complex that is? And in the absence of that approach, #193 is already an improvement!

@peterdesmet peterdesmet added the enhancement New feature or request label Apr 27, 2023
@peterdesmet
Copy link
Member

The pull request is now deployed, also on aloft (navigate to the next day). I will close this issue now:

  • User is informed that no data makes it to the chart (except when mixed)
  • In the console one can see 404s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants