[ARVADOS] created: be3aa4ed56f0e39776fd0b1e7bcb8f4c1465ba09

[git at public.curoverse.com]

        at  be3aa4ed56f0e39776fd0b1e7bcb8f4c1465ba09 (commit)


commit be3aa4ed56f0e39776fd0b1e7bcb8f4c1465ba09
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
--