GCM Engine: Split up reg/unreg UNKNOWN_ERROR to improve metrics

google_apis/gcm/engine/unregistration_request.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "google_apis/gcm/engine/unregistration_request.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/location.h"
@@ skipping 18 matching lines @@
 const char kRequestContentType[] = "application/x-www-form-urlencoded";
 
 // Request constants.
 const char kCategoryKey[] = "app";
 const char kSubtypeKey[] = "X-subtype";
 const char kDeleteKey[] = "delete";
 const char kDeleteValue[] = "true";
 const char kDeviceIdKey[] = "device";
 const char kLoginHeader[] = "AidLogin";
 
+// Response constants.
+const char kErrorPrefix[] = "Error=";
+const char kInvalidParameters[] = "INVALID_PARAMETERS";
+const char kInternalServerError[] = "InternalServerError";
+const char kPhoneRegistrationError[] = "PHONE_REGISTRATION_ERROR";

[Nicolas Zea, 2016/10/20 21:05:23]

nit: Given this maps to DEVICE_REGISTRATION_ERROR, I think it's cleaner to have
the variable name here b kDeviceRegistrationError

[johnme, 2016/11/01 14:15:40]

Done.

+
+// Gets correct status from the error message.
+UnregistrationRequest::Status GetStatusFromError(const std::string& error) {
+  if (error.find(kInvalidParameters) != std::string::npos)
+    return UnregistrationRequest::INVALID_PARAMETERS;
+  if (error.find(kInternalServerError) != std::string::npos)
+    return UnregistrationRequest::INTERNAL_SERVER_ERROR;
+  if (error.find(kPhoneRegistrationError) != std::string::npos)
+    return UnregistrationRequest::DEVICE_REGISTRATION_ERROR;
+  // Should not be reached, unless the server adds new error types.
+  return UnregistrationRequest::UNKNOWN_ERROR;
+}
+
 // Determines whether to retry based on the status of the last request.
 bool ShouldRetryWithStatus(UnregistrationRequest::Status status) {
   switch (status) {
     case UnregistrationRequest::URL_FETCHING_FAILED:
     case UnregistrationRequest::NO_RESPONSE_BODY:
     case UnregistrationRequest::RESPONSE_PARSING_FAILED:
     case UnregistrationRequest::INCORRECT_APP_ID:
     case UnregistrationRequest::SERVICE_UNAVAILABLE:
     case UnregistrationRequest::INTERNAL_SERVER_ERROR:
     case UnregistrationRequest::HTTP_NOT_OK:
       return true;
     case UnregistrationRequest::SUCCESS:
     case UnregistrationRequest::INVALID_PARAMETERS:
+    case UnregistrationRequest::DEVICE_REGISTRATION_ERROR:
     case UnregistrationRequest::UNKNOWN_ERROR:
     case UnregistrationRequest::REACHED_MAX_RETRIES:
       return false;
     case UnregistrationRequest::UNREGISTRATION_STATUS_COUNT:
       NOTREACHED();
       break;
   }
   return false;
 }
 
@@ skipping 90 matching lines @@
                     body);
   BuildFormEncoding(kDeleteKey, kDeleteValue, body);
 
   DCHECK(custom_request_handler_.get());
   custom_request_handler_->BuildRequestBody(body);
 }
 
 UnregistrationRequest::Status UnregistrationRequest::ParseResponse(
     const net::URLFetcher* source) {
   if (!source->GetStatus().is_success()) {
-    DVLOG(1) << "Fetcher failed";
+    DVLOG(1) << "Unregistration URL fetching failed.";
     return URL_FETCHING_FAILED;
   }
 
+  std::string response;
+  if (!source->GetResponseAsString(&response)) {
+    DVLOG(1) << "Failed to get unregistration response body.";
+    return NO_RESPONSE_BODY;
+  }
+
+  // If we are able to parse a meaningful known error, let's do so. Note that
+  // some errors will have HTTP_OK response code!
+  if (response.find(kErrorPrefix) != std::string::npos) {
+    std::string error = response.substr(
+        response.find(kErrorPrefix) + arraysize(kErrorPrefix) - 1);
+    DVLOG(1) << "Unregistration response error message: " << error;
+    return GetStatusFromError(error);
+  }
+
   net::HttpStatusCode response_status = static_cast<net::HttpStatusCode>(
       source->GetResponseCode());
   if (response_status != net::HTTP_OK) {
-    DVLOG(1) << "HTTP Status code is not OK, but: " << response_status;
+    DVLOG(1) << "Unregistration HTTP response code not OK: " << response_status;
     if (response_status == net::HTTP_SERVICE_UNAVAILABLE)
       return SERVICE_UNAVAILABLE;
     if (response_status == net::HTTP_INTERNAL_SERVER_ERROR)
       return INTERNAL_SERVER_ERROR;
     return HTTP_NOT_OK;
   }
 
   DCHECK(custom_request_handler_.get());
-  return custom_request_handler_->ParseResponse(source);
+  return custom_request_handler_->ParseResponse(response);
 }
 
 void UnregistrationRequest::RetryWithBackoff() {
   DCHECK_GT(retries_left_, 0);
   --retries_left_;
   url_fetcher_.reset();
   backoff_entry_.InformOfRequest(false);
 
   DVLOG(1) << "Delaying GCM unregistration of app: " << request_info_.app_id()
            << ", for " << backoff_entry_.GetTimeUntilRelease().InMilliseconds()
@@ skipping 35 matching lines @@
     // Only REACHED_MAX_RETRIES is reported because the function will skip
     // reporting count and time when status is not SUCCESS.
     DCHECK(custom_request_handler_.get());
     custom_request_handler_->ReportUMAs(status, 0, base::TimeDelta());
   }
 
   callback_.Run(status);
 }
 
 }  // namespace gcm