Bug #2891

Workbench error pages should be helpful

Added by Brett Smith over 5 years ago. Updated about 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Brett Smith
Category:
-
Target version:
Start date:
06/17/2014
Due date:
% Done:

100%

Estimated time:
(Total: 11.00 h)
Story points:
1.0

Description

Examples
  • A user recently tried to view a collection they did not have permission to read (see #2890). The API server returned 404 for this request, and Workbench rendered a 422 Fiddlesticks. Workbench should render a more-specific 404 for this specific error.
  • If an object is not found, but the desired/expected object type can be determined, offer a link to a "search all X" page.
  • Unexpected errors from the API server should be presented with some more boilerplate rather than just "blah [API 422]".
    • "Workbench received an error response from an Arvados API method. The details of the request and response follow:"
    • Show request URL (but avoid showing posted parameters, in case they are huge or include API tokens)
    • Show pretty-json-formatted API response in a PRE block
    • Show timestamp of request or response (and, perhaps better: on the API server side, include some sort of unique identifier in each error response, like timestamp+random, and send that to logger.error as well. Workbench can also log that ID when rendering an error page. This would make it much easier for a server admin to cross reference a user's error report / screen capture with the relevant parts of the Workbench and API log files.)
    • Suggest an appropriate next step. "It's possible that you can fix this by refreshing the page, but it's more likely a software bug: either API server is not behaving the way Workbench expects, or Workbench has made a mistake by offering to let you do something that isn't really possible. In that case, your best option is to report this information to the server administrator via {Rails.configuration.bug_reporting_contact_link}."
  • Error page should not give the appearance of being logged out if in fact the user is still logged in.
Workbench Errors 404.png (68.5 KB) Workbench Errors 404.png New 404 page with search link Brett Smith, 07/15/2014 05:58 PM
Workbench Errors 422.png (88.3 KB) Workbench Errors 422.png New Fiddlesticks page with error token Brett Smith, 07/15/2014 05:58 PM

Subtasks

Task #3074: Review 2891-workbench-errors-retain-login-wipResolvedPeter Amstutz

Task #3032: Error in rendering error.htmlResolvedBrett Smith

Task #3092: Review 2891-improve-workbench-errors-wipResolvedPeter Amstutz

Task #3080: Workbench "not found" page links to the object type indexResolvedBrett Smith

Task #3079: Render 404 for not found Collections and filesResolvedBrett Smith

Associated revisions

Revision f2756832
Added by Brett Smith about 5 years ago

Merge branch '2891-workbench-errors-retain-login'

Closes #3032, #3074. Refs #2891.

Revision fcb35ed4
Added by Brett Smith about 5 years ago

Merge branch '2891-improve-workbench-errors'

Closes #3079, #3080, #2891, #3092.

History

#1 Updated by Tom Clegg over 5 years ago

  • Subject changed from Workbench returns 422 for Collection not found to Workbench error pages should be helpful
  • Description updated (diff)

#2 Updated by Tom Clegg over 5 years ago

  • Target version set to 2014-07-16 Sprint

#3 Updated by Tom Clegg over 5 years ago

  • Story points set to 1.0

#4 Updated by Brett Smith about 5 years ago

  • Assigned To set to Brett Smith

#5 Updated by Brett Smith about 5 years ago

2891-workbench-errors-retain-login-wip is up for review. It only addresses task #3032 and the last bullet point of the story here, but I wanted to get it up for review early because it seemed like a useful bugfix before deployment.

#6 Updated by Tom Clegg about 5 years ago

  • Description updated (diff)

#7 Updated by Brett Smith about 5 years ago

2891-improve-workbench-errors-wip is up for review. This implements all of the story except the last point, which was addressed in a previous bugfix branch.

#8 Updated by Peter Amstutz about 5 years ago

Still getting red box of doom when it can't connect to API server (should give a more helpful error):

Started GET "/pipeline_instances/peter-d1hrv-ufg9mhsl55bd176" for 127.0.0.1 at 2014-06-26 14:48:36 -0400
Processing by PipelineInstancesController#show as HTML
  Parameters: {"id"=>"peter-d1hrv-ufg9mhsl55bd176"}
API client: 0.000282834 Prepare request https://localhost:4001/discovery/v1/apis/arvados/v1/rest   
#<Errno::ECONNREFUSED: Connection refused - connect(2) for "localhost" port 4001 (https://localhost:4001)>
.
.
.
  Rendered application/error.html.erb within layouts/application (0.7ms)
Completed 500 Internal Server Error in 17ms

ActionView::Template::Error (unknown attribute: uuid):
    14:   <link rel="shortcut icon" href="/favicon.ico" type="image/x-icon">
    15:   <meta name="description" content="">
    16:   <meta name="author" content="">
    17:   <% if current_user and $arvados_api_client.discovery[:websocketUrl] %>
    18:   <meta name="arv-websocket-url" content="<%=$arvados_api_client.discovery[:websocketUrl]%>?api_token=<%=Thread.current[:arvados_api_token]%>">
    19:   <% end %>
    20:   <meta name="robots" content="NOINDEX, NOFOLLOW">
  app/models/arvados_base.rb:34:in `initialize'
  app/models/user.rb:3:in `initialize'
  app/controllers/application_controller.rb:281:in `current_user'
  app/helpers/application_helper.rb:3:in `current_user'
  app/views/layouts/application.html.erb:17:in `_app_views_layouts_application_html_erb___3023445212025765766_42770440'
  app/controllers/application_controller.rb:46:in `block (2 levels) in render_error'
  app/controllers/application_controller.rb:40:in `render_error'
  app/controllers/application_controller.rb:68:in `block in render_exception'
  app/controllers/application_controller.rb:419:in `set_thread_api_token'
  app/controllers/application_controller.rb:67:in `render_exception'

#9 Updated by Peter Amstutz about 5 years ago

I tried accessing a pipeline template with a valid, non-existing UUID. Instead of a 404 I got a fiddlesticks:

Oh... fiddlesticks.
Sorry, I had some trouble handling your request.
  * undefined method `pipelineinstances_path' for #<#<Class:0x00000004c46810>:0x007f7358f366a0>

#10 Updated by Brett Smith about 5 years ago

Both bugs fixed as of 99832dd

#11 Updated by Peter Amstutz about 5 years ago

Oh... fiddlesticks.

Sorry, I had some trouble handling your request.

    undefined method `uuid' for nil:NilClass

Stack trace:

Started GET "/" for 127.0.0.1 at 2014-06-27 08:57:41 -0400
Processing by ProjectsController#index as HTML
API client: 0.000434774 Prepare request https://localhost:3001/arvados/v1/groups  {} [["group_class","in",["project","folder"]]]
API client: 0.078885059 API transaction
API client: 0.033231487 request_time
API client: 0.000352689 Parse response
  Rendered projects/index.html.erb within layouts/application (88.7ms)
#<NoMethodError: undefined method `uuid' for nil:NilClass>
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:588:in `block in my_projects'
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:587:in `select'
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:587:in `my_projects'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/abstract_controller/helpers.rb:70:in `my_projects'
/home/peter/work/arvados.secondary/apps/workbench/app/views/projects/index.html.erb:9:in `_app_views_projects_index_html_erb___2710018873239097020_70317208753140'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionview-4.1.1/lib/action_view/template.rb:145:in `block in render'

On brief inspection, it appears that current_user is nil. This happens after logging in. Clicking on the "Log in" link produces the same result.

#12 Updated by Brett Smith about 5 years ago

Fixed with two commits, now at d3d2311. Please take another look.

#13 Updated by Peter Amstutz about 5 years ago

  • Assigned To deleted (Brett Smith)

Error noted in comment #9 still happens.

Accessing a non-existent group has a similar bug:

Oh... fiddlesticks.

Sorry, I had some trouble handling your request.

    undefined method `group_class' for nil:NilClass

Started GET "/groups/bogus-j7d0g-000000000000001" for 127.0.0.1 at 2014-06-27 15:33:26 -0400
Processing by GroupsController#show as HTML
  Parameters: {"id"=>"bogus-j7d0g-000000000000001"}
API client: 0.000275893 Prepare request https://localhost:3001/arvados/v1/authorized_keys  {"authorized_user_uuid":"peter-tpzed-ufmnxhte5djhbq2"} 
API client: 0.316003509 API transaction
API client: 0.257994855 request_time
API client: 0.000173787 Parse response
API client: 0.000373739 Prepare request https://localhost:3001/arvados/v1/collections  {"created_by":"peter-tpzed-ufmnxhte5djhbq2"} 
API client: 0.077106156 API transaction
API client: 0.031651701 request_time
API client: 0.00023519 Parse response
API client: 0.000512045 Prepare request https://localhost:3001/arvados/v1/pipeline_instances  {"created_by":"peter-tpzed-ufmnxhte5djhbq2"} 
API client: 0.199535894 API transaction
API client: 0.099248624 request_time
API client: 0.000867306 Parse response
API client: 0.000398544 Prepare request https://localhost:3001/arvados/v1/groups/bogus-j7d0g-000000000000001   
API client: 0.068171705 API transaction
#<NoMethodError: undefined method `group_class' for nil:NilClass>
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/groups_controller.rb:11:in `show'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/action_controller/metal/implicit_render.rb:4:in `send_action'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/abstract_controller/base.rb:189:in `process_action'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/action_controller/metal/rendering.rb:10:in `process_action'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/abstract_controller/callbacks.rb:20:in `block in process_action'

#14 Updated by Peter Amstutz about 5 years ago

On the "Not Found" page, it suggests that you might want to browse the index page for type of thing you tried to access. However, it seems to lift this from the type field of the UUID and not from the controller, which leads to the following confusing sequence:

  1. Access a valid UUID (for example peter-j7d0g-000000000000000 is a system group) with an invalid controller, for example /nodes/peter-j7d0g-000000000000000
  2. It will suggest you browse all groups (instead of browse all nodes)
  3. The item I just asked for, that it said didn't exist, shows up right there in the index page.

#15 Updated by Peter Amstutz about 5 years ago

I'm still getting the red box of death when I deliberately kill the API server.

Started GET "/projects/peter-j7d0g-buoji82mhj29ipv" for 127.0.0.1 at 2014-06-27 15:45:29 -0400
Processing by ProjectsController#show as HTML
  Parameters: {"id"=>"peter-j7d0g-buoji82mhj29ipv"}
API client: 0.000474647 Prepare request https://localhost:3001/arvados/v1/authorized_keys  {"authorized_user_uuid":"peter-tpzed-ufmnxhte5djhbq2"} 
#<ArvadosApiClient::NoApiResponseException: Errno::ECONNREFUSED error connecting to API server>
/home/peter/work/arvados.secondary/apps/workbench/app/models/arvados_api_client.rb:128:in `rescue in block in api'
/home/peter/work/arvados.secondary/apps/workbench/app/models/arvados_api_client.rb:125:in `block in api'
/home/peter/work/arvados.secondary/apps/workbench/app/models/arvados_api_client.rb:124:in `synchronize'
/home/peter/work/arvados.secondary/apps/workbench/app/models/arvados_api_client.rb:124:in `api'
/home/peter/work/arvados.secondary/apps/workbench/app/models/arvados_resource_list.rb:69:in `where'
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:496:in `block in <class:ApplicationController>'
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:540:in `call'
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:540:in `block in check_user_notifications'
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:539:in `each'
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/application_controller.rb:539:in `check_user_notifications'
/home/peter/.rvm/gems/ruby-2.1.1/gems/activesupport-4.1.1/lib/active_support/callbacks.rb:424:in `block in make_lambda'
.
.
.
  Rendered application/api_error.html.erb within layouts/application (1.0ms)
API client: 0.000214136 Prepare request https://localhost:3001/arvados/v1/groups  {} [["group_class","in",["project","folder"]]]
  Rendered application/_projects_tree_menu.html.erb (2.5ms)
Completed 500 Internal Server Error in 18ms

ActionView::Template::Error (Errno::ECONNREFUSED error connecting to API server):
     8:                 <%= top_button.call %>
     9:                 My projects
    10:               </li>
    11:               <% my_project_tree.each do |pnode| %>
    12:                 <% next if pnode[:object].class != Group %>
    13:                 <li style="padding-left: <%= pnode[:depth]-1 %>em">
    14:                   <%= project_link_to.call pnode do %>
  app/models/arvados_api_client.rb:128:in `rescue in block in api'
  app/models/arvados_api_client.rb:125:in `block in api'
  app/models/arvados_api_client.rb:124:in `synchronize'
  app/models/arvados_api_client.rb:124:in `api'
  app/models/arvados_resource_list.rb:69:in `where'
  app/models/arvados_resource_list.rb:75:in `results'
  app/models/arvados_resource_list.rb:88:in `each'
  app/controllers/application_controller.rb:615:in `build_project_trees'
  app/controllers/application_controller.rb:602:in `my_project_tree'
  app/views/application/_projects_tree_menu.html.erb:11:in `_app_views_application__projects_tree_menu_html_erb__1266463543615626660_20841800'
  app/views/layouts/application.html.erb:163:in `_app_views_layouts_application_html_erb__2965800553768113_39471500'
  app/controllers/application_controller.rb:46:in `block (2 levels) in render_error'
  app/controllers/application_controller.rb:40:in `render_error'
  app/controllers/application_controller.rb:69:in `render_exception'

#16 Updated by Peter Amstutz about 5 years ago

Stack trace for #9

Started GET "/pipeline_templates/peter-p5p6p-95afkloceizouk9" for 127.0.0.1 at 2014-06-27 15:50:37 -0400
Processing by PipelineTemplatesController#show as HTML
  Parameters: {"id"=>"peter-p5p6p-95afkloceizouk9"}
API client: 0.000237838 Prepare request https://localhost:3001/arvados/v1/authorized_keys  {"authorized_user_uuid":"peter-tpzed-ufmnxhte5djhbq2"} 
API client: 0.038473034 API transaction
API client: 0.032112086 request_time
API client: 0.00016426 Parse response
API client: 0.000347903 Prepare request https://localhost:3001/arvados/v1/collections  {"created_by":"peter-tpzed-ufmnxhte5djhbq2"} 
API client: 0.084111913 API transaction
API client: 0.042941854 request_time
API client: 0.00018248 Parse response
API client: 0.000424665 Prepare request https://localhost:3001/arvados/v1/pipeline_instances  {"created_by":"peter-tpzed-ufmnxhte5djhbq2"} 
API client: 0.149595119 API transaction
API client: 0.062132264 request_time
API client: 0.000779794 Parse response
API client: 0.000368567 Prepare request https://localhost:3001/arvados/v1/pipeline_templates/peter-p5p6p-95afkloceizouk9   
API client: 0.066503744 API transaction
#<NoMethodError: undefined method `uuid' for nil:NilClass>
/home/peter/work/arvados.secondary/apps/workbench/app/controllers/pipeline_templates_controller.rb:4:in `show'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/action_controller/metal/implicit_render.rb:4:in `send_action'
/home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/abstract_controller/base.rb:189:in `process_action'

#17 Updated by Brett Smith about 5 years ago

  • Assigned To set to Brett Smith
  • c92fbef6 changes find_object_by_uuid to render the 404 page itself, rather than setting @object to nil. I had to make a small change to CollectionsController to accommodate this change, but that's easier than trying to play whack-a-mole with the different controllers whose show method assumes @object is not nil. This should address all the NoMethodErrors we've seen so far.
  • 93b56a17 uses the controller object to generate an index link, instead of the object UUID, to address #14.
  • 2c055a47 pre-builds the object tree so that it doesn't fail during template rendering and cause a 500. I'm sorry this has taken so long to nail down. It turns out it's apparently timing-sensitive: if you wait just a little bit after bringing the API server down, Workbench will try and fail to refresh the discovery document, which causes it to successfully render an error page without any user information. So I had to be faster to reproduce what you were seeing.

Ready for another look at 2c055a47. Thanks.

#18 Updated by Peter Amstutz about 5 years ago

I'm being persnickety, but these should be easy to fix:

1) I would like the 404 page to include the request path that wasn't found, so the user doesn't have to rely on the URL bar (which is grayed out or hidden in some browsers)

2) When accessing an object with a nonconforming uuid:

Oh... fiddlesticks.

Sorry, I had some trouble handling your request.

    argument to find() must be a uuid string. Acceptable formats: warehouse locator or string with format xxxxx-xxxxx-xxxxxxxxxxxxxxx

Arguably this should be a 404, not a fiddlesticks. Also it should know whether is it rendering under the collections controller (expecting a keep locator) or another controller (expecting an arvados uuid) and present the relevant error message.

#19 Updated by Brett Smith about 5 years ago

Peter Amstutz wrote:

I'm being persnickety, but these should be easy to fix:

1) I would like the 404 page to include the request path that wasn't found, so the user doesn't have to rely on the URL bar (which is grayed out or hidden in some browsers)

I added the item class and requested UUID to the 404 page in this case. I would like to avoid rendering URLs as text when possible, just because it's not pretty. I believe this still provides the desired information.

2) When accessing an object with a nonconforming uuid:

[...]

Arguably this should be a 404, not a fiddlesticks. Also it should know whether is it rendering under the collections controller (expecting a keep locator) or another controller (expecting an arvados uuid) and present the relevant error message.

Made this case a 404. It doesn't go into detail about the UUID format, because, why should the user care?

Ready for review at 5abf8f7. Thanks.

#20 Updated by Peter Amstutz about 5 years ago

Looks good to me. Please merge.

#21 Updated by Brett Smith about 5 years ago

  • Status changed from New to Resolved
  • % Done changed from 85 to 100

Applied in changeset arvados|commit:fcb35ed42449c5ad58cd9ec62ff2ed353cf9d940.

Also available in: Atom PDF