Arvados

Warning, #3170 is moving the tab-loading javascript around a bit.

Branch 3200-ajax-error-handling-wip is up for review. Not all the changes are related, so:

• The fail callback added to tab_panes.js shows an error when a tab fails to load, to resolve #3276.

• The changes to the error function in editable.js helps make sure that X-Editable always shows an error, even when the response doesn't look the way we expect. This helps both #3207 and #3251.

• The changes to Workbench's API client help ensure that errors from API requests get passed on correctly to Workbench AJAX requests. This provides a better error message for #3207, at least, and probably other cases that aren't known yet.

Review completed. Everything looks and works great. Just a couple very minor suggestions.

- The tab error message seems verbose. And, it feels like we could rearrange the message to make it easy to work with. How about something like:

An error occurred while loading this page: Errno::ECONNREFUSED error connecting to API server

RELOAD BUTTON

Please try reloading this page. If the problem persists, the information below . . .

- Reload button title: Can it be just “Reload” instead of “Reload tab”?

Radhika Chippada wrote:

- The tab error message seems verbose. And, it feels like we could rearrange the message to make it easy to work with. How about something like:

Which error message are you referring to specifically? If Workbench returns an error page when rendering the tab, this branch shows that error page verbatim. And having the error page with extra information about failures to connect to the API server was something we decided in a previous story, #2891.

I'm open to the idea of rearranging the layout, but the fact that the best thing we can do is render the whole response page in one blob makes it a little tricky. For example, in your suggestion:

An error occurred while loading this page: Errno::ECONNREFUSED error connecting to API server

This isn't doable, because the rendering Javascript doesn't know that the error was ECONNREFUSED. That information is in the HTML response, which the Javascript has to render all together.

Is there another improvement we might make?

- Reload button title: Can it be just “Reload” instead of “Reload tab”?

I'm okay with making it shorter, but I'm a little concerned users might think it reloads the whole page. Do you think that's all right?

Thanks.

My comments about the error message were when tab reloading failed (such as the project related tabs "Other objects" etc).

If it is harder to rearrange the text, I think it would be nice if we could do something like:

RELOAD button
   Then the message.

I think having the reload button early on in the tab would be more usable, especially for someone who has seen similar error before.

Thanks for the feedback. I just pushed 09f5240, which puts a generic "An error occurred" message and the reload button at the top of the error page. Please take another look.

Completed reviewing again. Suggested to Brett that we should just remove the generic "an error occurred" text to make it look better.