Merge branch '15318-ReqIds-on-api-errors'

Closes #15318

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima@veritasgenetics.com>

diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index f7735a3053fdd377b00d5b2e8097375d49910237..169b1f79614bd9b0391542ae71771dac87e0d95d 100644 (file)
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -134,7 +134,7 @@ func (s *FederationSuite) TestNoAuth(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
        resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized)
-       s.checkJSONErrorMatches(c, resp, `Not logged in`)
+       s.checkJSONErrorMatches(c, resp, `Not logged in.*`)
 }
 
 func (s *FederationSuite) TestBadAuth(c *check.C) {
@@ -142,7 +142,7 @@ func (s *FederationSuite) TestBadAuth(c *check.C) {
        req.Header.Set("Authorization", "Bearer aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
        resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized)
-       s.checkJSONErrorMatches(c, resp, `Not logged in`)
+       s.checkJSONErrorMatches(c, resp, `Not logged in.*`)
 }
 
 func (s *FederationSuite) TestNoAccess(c *check.C) {
@@ -150,7 +150,7 @@ func (s *FederationSuite) TestNoAccess(c *check.C) {
        req.Header.Set("Authorization", "Bearer "+arvadostest.SpectatorToken)
        resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
-       s.checkJSONErrorMatches(c, resp, `.*not found`)
+       s.checkJSONErrorMatches(c, resp, `.*not found.*`)
 }
 
 func (s *FederationSuite) TestGetUnknownRemote(c *check.C) {

diff --git a/lib/dispatchcloud/container/queue_test.go b/lib/dispatchcloud/container/queue_test.go
index daf7977ad50c5af0380ee44282bf81ff439e4d3b..e817a0cc7b65459ac753394aae0c3eb503303e59 100644 (file)
--- a/lib/dispatchcloud/container/queue_test.go
+++ b/lib/dispatchcloud/container/queue_test.go
@@ -74,7 +74,7 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
                        defer wg.Done()
                        err := cq.Unlock(uuid)
                        c.Check(err, check.NotNil)
-                       c.Check(err, check.ErrorMatches, ".*cannot unlock when Queued*.")
+                       c.Check(err, check.ErrorMatches, ".*cannot unlock when Queued.*")
 
                        err = cq.Lock(uuid)
                        c.Check(err, check.IsNil)

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index e07a5aca79b5a310d320c6901df9c5a82326465a..d5bc3f35d7e04fc47f9795f8a3643f4aef8bcf57 100644 (file)
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -183,6 +183,9 @@ class ApplicationController < ActionController::Base
       err = {}
     end
     err[:errors] ||= args
+    err[:errors].map! do |err|
+      err += " (" + Thread.current[:request_id] + ")"
+    end
     err[:error_token] = [Time.now.utc.to_i, "%08x" % rand(16 ** 8)].join("+")
     status = err.delete(:status) || 422
     logger.error "Error #{err[:error_token]}: #{status}"

diff --git a/services/api/test/functional/application_controller_test.rb b/services/api/test/functional/application_controller_test.rb
index b5f71acb536aa9bdd2bbf1ba67bbd6aba55560af..b74ff0f41d6d5190f217df7cc776490d0e8e9d83 100644 (file)
--- a/services/api/test/functional/application_controller_test.rb
+++ b/services/api/test/functional/application_controller_test.rb
@@ -24,11 +24,16 @@ class ApplicationControllerTest < ActionController::TestCase
     token_time = token.split('+', 2).first.to_i
     assert_operator(token_time, :>=, @start_stamp, "error token too old")
     assert_operator(token_time, :<=, now_timestamp, "error token too new")
+    json_response['errors'].each do |err|
+      assert_match(/req-[a-z0-9]{20}/, err, "X-Request-Id value missing on error message")
+    end
   end
 
   def check_404(errmsg="Path not found")
     assert_response 404
-    assert_equal([errmsg], json_response['errors'])
+    json_response['errors'].each do |err|
+      assert(err.include?(errmsg), "error message '#{err}' expected to include '#{errmsg}'")
+    end
     check_error_token
   end
 

diff --git a/services/api/test/integration/login_workflow_test.rb b/services/api/test/integration/login_workflow_test.rb
index 8691030e9d3c09e14a909ae0f0a3bd647a293212..f0741fcfde9f2fe27297720f5bbc4bb88af9b418 100644 (file)
--- a/services/api/test/integration/login_workflow_test.rb
+++ b/services/api/test/integration/login_workflow_test.rb
@@ -10,7 +10,9 @@ class LoginWorkflowTest < ActionDispatch::IntegrationTest
       params: {specimen: {}},
       headers: {'HTTP_ACCEPT' => ''})
     assert_response 401
-    assert_includes(json_response['errors'], "Not logged in")
+    json_response['errors'].each do |err|
+      assert(err.include?("Not logged in"), "error message '#{err}' expected to include 'Not logged in'")
+    end
   end
 
   test "login prompt respects JSON Accept header" do
@@ -18,7 +20,9 @@ class LoginWorkflowTest < ActionDispatch::IntegrationTest
       params: {specimen: {}},
       headers: {'HTTP_ACCEPT' => 'application/json'})
     assert_response 401
-    assert_includes(json_response['errors'], "Not logged in")
+    json_response['errors'].each do |err|
+      assert(err.include?("Not logged in"), "error message '#{err}' expected to include 'Not logged in'")
+    end
   end
 
   test "login prompt respects HTML Accept header" do