[ARVADOS] updated: 8864cde017b9214b2e5383397b182f0b5f60334c

git at public.curoverse.com git at public.curoverse.com
Fri Aug 15 16:44:05 EDT 2014


Summary of changes:
 .../app/controllers/application_controller.rb      | 36 ++++++++++++++++------
 .../test/functional/application_controller_test.rb | 18 +++++++++++
 2 files changed, 45 insertions(+), 9 deletions(-)

       via  8864cde017b9214b2e5383397b182f0b5f60334c (commit)
       via  d3427aea4532d247b71b5ebfc5f5eb527e8835a2 (commit)
      from  b5a278786165ebbbd63044cf9b5b64b949fe88ed (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 8864cde017b9214b2e5383397b182f0b5f60334c
Merge: b5a2787 d3427ae
Author: Brett Smith <brett at curoverse.com>
Date:   Fri Aug 15 16:45:21 2014 -0400

    Merge branch '3593-workbench-exception-rendering'
    
    Closes #3593, #3595.


commit d3427aea4532d247b71b5ebfc5f5eb527e8835a2
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Aug 14 13:59:18 2014 -0400

    3593: Another round of hardening Workbench's render_exception.
    
    render_exception was counting on session[:user] to help render the
    error page.  We recently excised that, which is turning exceptions
    into 500s.  This commit restores the session user object, but uses it
    only for error rendering, in keeping with the spirit of the previous
    revert.  This commit also defends against more possible failure cases
    in render_exception, and adds a basic test for it.

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 2228880..bbc2a82 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -62,22 +62,31 @@ class ApplicationController < ActionController::Base
     else
       @errors = [e.to_s]
     end
-    # If the user has an active session, and the API server is available,
-    # make user information available on the error page.
+    # Make user information available on the error page, falling back to the
+    # session cache if the API server is unavailable.
     begin
       load_api_token(session[:arvados_api_token])
     rescue ArvadosApiClient::ApiError
-      load_api_token(nil)
+      unless session[:user].nil?
+        begin
+          Thread.current[:user] = User.new(session[:user])
+        rescue ArvadosApiClient::ApiError
+          # This can happen if User's columns are unavailable.  Nothing to do.
+        end
+      end
     end
-    # Preload projects trees for the template.  If that fails, set empty
+    # Preload projects trees for the template.  If that's not doable, set empty
     # trees so error page rendering can proceed.  (It's easier to rescue the
     # exception here than in a template.)
-    begin
-      build_project_trees
-    rescue ArvadosApiClient::ApiError
-      @my_project_tree ||= []
-      @shared_project_tree ||= []
+    unless current_user.nil?
+      begin
+        build_project_trees
+      rescue ArvadosApiClient::ApiError
+        # Fall back to the default-setting code later.
+      end
     end
+    @my_project_tree ||= []
+    @shared_project_tree ||= []
     render_error(err_opts)
   end
 
@@ -427,6 +436,15 @@ class ApplicationController < ActionController::Base
       false  # We may redirect to login, or not, based on the current action.
     else
       session[:arvados_api_token] = params[:api_token]
+      session[:user] = {
+        uuid: user.uuid,
+        email: user.email,
+        first_name: user.first_name,
+        last_name: user.last_name,
+        is_active: user.is_active,
+        is_admin: user.is_admin,
+        prefs: user.prefs
+      }
 
       if !request.format.json? and request.method.in? ['GET', 'HEAD']
         # Repeat this request with api_token in the (new) session
diff --git a/apps/workbench/test/functional/application_controller_test.rb b/apps/workbench/test/functional/application_controller_test.rb
index 50f990a..c282802 100644
--- a/apps/workbench/test/functional/application_controller_test.rb
+++ b/apps/workbench/test/functional/application_controller_test.rb
@@ -303,4 +303,22 @@ class ApplicationControllerTest < ActionController::TestCase
     get(:show, {id: "zzzzz-zzzzz-zzzzzzzzzzzzzzz"}, session_for(:admin))
     assert_response 404
   end
+
+  test "Workbench returns 4xx when API server is unreachable" do
+    # We're really testing ApplicationController's render_exception.
+    # Our primary concern is that it doesn't raise an error and
+    # return 500.
+    orig_api_server = Rails.configuration.arvados_v1_base
+    begin
+      # The URL should look valid in all respects, and avoid talking over a
+      # network.  100::/64 is the IPv6 discard prefix, so it's perfect.
+      Rails.configuration.arvados_v1_base = "https://[100::f]:1/"
+      @controller = NodesController.new
+      get(:index, {}, session_for(:active))
+      assert_includes(405..422, @response.code.to_i,
+                      "bad response code when API server is unreachable")
+    ensure
+      Rails.configuration.arvados_v1_base = orig_api_server
+    end
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list