search

NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 27 forks source link

MDQ chart needs to fail gracefully #1249

Closed laurenwalker closed 4 years ago

laurenwalker commented 4 years ago

When the MDQ image fails to render for me, the loading spinner is displayed forever. The chart needs to display some kind of feedback to the user when it has failed to render. (I thought that is why we chose to use Promise?)

It would be good to show a message that explains what the reason mostly likely is, and any steps that can be taken to remedy it.

It's possible it is failing to show the error message for me because of a domino-effect from the issue in #1248, but those cases need to be caught, too, since all the other charts on the page look fine.

Screen Shot 2020-01-20 at 6 06 31 PM

gothub commented 4 years ago

@laurenwalker - the error handling is currently in Stats.js and StatsView.js. Here is the error dialog that is displayed when I intentionally cause the server to fail by misspelling the url in the AppModel.js attribute:

Screen Shot 2020-01-21 at 3 34 43 PM

The dialog is only displayed for a few seconds. I used the parameters found in other views:


                MetacatUI.appView.showAlert("Metadata Assessment Metrics are not available for this collection: " + msg, 
                    "alert-error", this.$("#metadata-assesment-graphic"), 4000, {remove: true});
``

Now that #1248 is resolved, the error mechanism should be working properly, as now there is not the internal error of attributes not being defined.
laurenwalker commented 4 years ago

This is a failure coming from a different part of the code than what you are referencing.

The error I experienced (which is now fixed since you added the mdq attributes to the default AppModel), was not showing the error message since it occurred in Stats.getMdqScores().

This can easily be fixed by wrapping the Stats.getMdqScores() function in a try, catch statement (which we should be doing for every function anyway), where in the catch statement, we trigger an error event that the view can listen to and show the error message.

Related: I think the error message you've screenshot looks really strange. I created an issue a while ago for this: #1205. I think that needs to be fixed for 2.9.0 as well. I can try to get to that myself if I can, but there are a lot of 2.9.0 issues so if you have the time to fix it, that would be great