Convert 4xx ServiceControl failures to 500

src/api_manager/service_control/aggregated.cc

@@ -93,12 +93,10 @@ const char servicecontrol_service[] =
 const char quotacontrol_service[] =
     "/google.api.servicecontrol.v1.QuotaController";
 
-// Define network failure error codes:
-// All 5xx Http status codes are marked as network failure.
 // Http status code is converted to Status::code as:
 // https://github.com/cloudendpoints/esp/blob/master/src/api_manager/utils/status.cc#L364
 // which is called by Status.ToProto() at Aggregated::Call() on_done function.
-bool IsErrorCodeNetworkFailure(int code) {
+bool StatusCodeIs5xxHttpCode(int code) {
   return code == Code::UNAVAILABLE || code == Code::INTERNAL ||
          code == Code::UNIMPLEMENTED || code == Code::DEADLINE_EXCEEDED;
 }
@@ -381,18 +379,29 @@ void Aggregated::Check(
           *response, service_control_proto_.service_name(), &response_info);
       on_done(status, response_info);
     } else {
-      // If network_fail_open is true, it is OK to proceed
-      if (network_fail_open_ &&
-          IsErrorCodeNetworkFailure(status.error_code())) {
+      // All 5xx Http status codes are treated as network failure.
+      // If network_fail_open is true, it is OK to proceed with these errors.
+      if (network_fail_open_ && StatusCodeIs5xxHttpCode(status.error_code())) {
         env_->LogError(
             std::string("With network fail open policy, the request is allowed "
                         "even the service control check failed with: " +
                         status.ToString()));
         on_done(Status::OK, response_info);
       } else {
-        on_done(Status(status.error_code(), status.error_message(),
-                       Status::SERVICE_CONTROL),
-                response_info);
+        // Preserve ServiceControl 5xx Http response code, especially 500 and
+        // 503. Convert non-5xx Http resonse code to 500 since they are most
+        // likely from API producer config errors. For example, wrong service
+        // account permission, or some key services are not enabled.
+        int error_code;
+        if (StatusCodeIs5xxHttpCode(status.error_code())) {
+          error_code = status.error_code();
+        } else {
+          error_code = Code::INTERNAL;
+        }
+
+        on_done(
+            Status(error_code, status.error_message(), Status::SERVICE_CONTROL),
+            response_info);
       }
     }
     delete response;

src/nginx/t/check_return_403.t

@@ -97,7 +97,7 @@ my $response = ApiManager::http_get($NginxPort,'/shelves?key=this-is-an-api-key'
 $t->stop_daemons();
 
 my ($response_headers, $response_body) = split /\r\n\r\n/, $response, 2;
-like($response, qr/HTTP\/1\.1 403 Forbidden/, 'Returned HTTP 403');
+like($response, qr/HTTP\/1\.1 500 Internal Server Error/, 'Returned HTTP 500');
 like($response, qr/detail error message line1/, 'error body line 1 is included');
 like($response, qr/detail error message line2/, 'error body line 2 is included');
 

src/nginx/t/failed_check.t

@@ -99,12 +99,12 @@ like($response1, qr/content-type: application\/json/i,
 like($response1, qr/API endpoints-test.cloudendpointsapis.com is not enabled for the consumer project/i,
   "Error body contains 'activation error'.");
 
-like($response2, qr/HTTP\/1\.1 403 Forbidden/, 'Response2 returned HTTP 403.');
+like($response2, qr/HTTP\/1\.1 500 Internal Server Error/, 'Response2 returned HTTP 500.');
 like($response2, qr/content-type: application\/json/i,
      'Unauthorized returned application/json body.');
 like($response2, qr/Service control request failed/i, "Error body contains 'service control failed'.");
 
-like($response3, qr/HTTP\/1\.1 400 Bad Request/, 'Response3 returned HTTP 400.');
+like($response3, qr/HTTP\/1\.1 500 Internal Server Error/, 'Response3 returned HTTP 500.');
 like($response3, qr/content-type: application\/json/i,
      'Unauthorized returned application/json body.');
 

src/nginx/t/report_failed_request.t

@@ -92,7 +92,7 @@ $t->stop_daemons();
 
 my ($response_headers, $response_body) = split /\r\n\r\n/, $response, 2;
 
-like($response_headers, qr/HTTP\/1\.1 403 Forbidden/, 'Returned HTTP 403.');
+like($response_headers, qr/HTTP\/1\.1 500 Internal Server Error/, 'Returned HTTP 500.');
 
 my @servicecontrol_requests = ApiManager::read_http_stream($t, 'servicecontrol.log');
 is(scalar @servicecontrol_requests, 2, 'Service control was called twice.');
@@ -113,14 +113,14 @@ my $expected_report_body = ServiceControl::gen_report_body({
   'api_key' => 'this-is-an-api-key',
   'api_method' =>  'ListShelves',
   'http_method' => 'GET',
-  'log_message' => 'Method: ListShelves failed: PERMISSION_DENIED: Service control request failed with HTTP response code 403',
-  'response_code' => '403',
+  'log_message' => 'Method: ListShelves failed: INTERNAL: Service control request failed with HTTP response code 403',
+  'response_code' => '500',
   'error_cause' => 'service_control',
-  'error_type' => '4xx',
+  'error_type' => '5xx',
   'request_size' => 62,
-  'response_size' => 380,
+  'response_size' => 367,
   'request_bytes' => 62,
-  'response_bytes' => 380,
+  'response_bytes' => 367,
   'producer_project_id' => 'endpoints-test',
   });