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

google_apis/gcm/engine/registration_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/registration_request.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 27 matching lines @@
 const char kDeviceIdKey[] = "device";
 const char kLoginHeader[] = "AidLogin";
 
 // Response constants.
 const char kErrorPrefix[] = "Error=";
 const char kTokenPrefix[] = "token=";
 const char kDeviceRegistrationError[] = "PHONE_REGISTRATION_ERROR";
 const char kAuthenticationFailed[] = "AUTHENTICATION_FAILED";
 const char kInvalidSender[] = "INVALID_SENDER";
 const char kInvalidParameters[] = "INVALID_PARAMETERS";
+const char kInternalServerError[] = "InternalServerError";

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

Is this defined server-side? Seems odd that the format is CamelCase rather than
ALL_CAPS

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

Yup, this one just happens to be CamelCase for no apparent reason:
https://goto.google.com/iepyy

+const char kQuotaExceeded[] = "QUOTA_EXCEEDED";
+const char kTooManyRegistrations[] = "TOO_MANY_REGISTRATIONS";
 
 // Gets correct status from the error message.
 RegistrationRequest::Status GetStatusFromError(const std::string& error) {
-  // TODO(fgorski): Improve error parsing in case there is nore then just an
-  // Error=ERROR_STRING in response.
   if (error.find(kDeviceRegistrationError) != std::string::npos)
     return RegistrationRequest::DEVICE_REGISTRATION_ERROR;
   if (error.find(kAuthenticationFailed) != std::string::npos)
     return RegistrationRequest::AUTHENTICATION_FAILED;
   if (error.find(kInvalidSender) != std::string::npos)
     return RegistrationRequest::INVALID_SENDER;
   if (error.find(kInvalidParameters) != std::string::npos)
     return RegistrationRequest::INVALID_PARAMETERS;
+  if (error.find(kInternalServerError) != std::string::npos)
+    return RegistrationRequest::INTERNAL_SERVER_ERROR;
+  if (error.find(kQuotaExceeded) != std::string::npos)
+    return RegistrationRequest::QUOTA_EXCEEDED;
+  if (error.find(kTooManyRegistrations) != std::string::npos)
+    return RegistrationRequest::TOO_MANY_REGISTRATIONS;
+  // Should not be reached, unless the server adds new error types.
   return RegistrationRequest::UNKNOWN_ERROR;
 }
 
 // Determines whether to retry based on the status of the last request.
 bool ShouldRetryWithStatus(RegistrationRequest::Status status) {
   switch (status) {
     case RegistrationRequest::AUTHENTICATION_FAILED:
     case RegistrationRequest::DEVICE_REGISTRATION_ERROR:
     case RegistrationRequest::UNKNOWN_ERROR:
     case RegistrationRequest::URL_FETCHING_FAILED:
     case RegistrationRequest::HTTP_NOT_OK:
     case RegistrationRequest::NO_RESPONSE_BODY:
+    case RegistrationRequest::RESPONSE_PARSING_FAILED:
+    case RegistrationRequest::INTERNAL_SERVER_ERROR:
       return true;
     case RegistrationRequest::SUCCESS:
     case RegistrationRequest::INVALID_PARAMETERS:
     case RegistrationRequest::INVALID_SENDER:
+    case RegistrationRequest::QUOTA_EXCEEDED:
+    case RegistrationRequest::TOO_MANY_REGISTRATIONS:
     case RegistrationRequest::REACHED_MAX_RETRIES:
       return false;
     case RegistrationRequest::STATUS_COUNT:
       NOTREACHED();
       break;
   }
   return false;
 }
 
 }  // namespace
@@ skipping 105 matching lines @@
   DCHECK(!weak_ptr_factory_.HasWeakPtrs());
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&RegistrationRequest::Start, weak_ptr_factory_.GetWeakPtr()),
       backoff_entry_.GetTimeUntilRelease());
 }
 
 RegistrationRequest::Status RegistrationRequest::ParseResponse(
     const net::URLFetcher* source, std::string* token) {
   if (!source->GetStatus().is_success()) {
-    LOG(ERROR) << "URL fetching failed.";
+    DVLOG(1) << "Registration URL fetching failed.";
     return URL_FETCHING_FAILED;
   }
 
   std::string response;
   if (!source->GetResponseAsString(&response)) {
-    LOG(ERROR) << "Failed to parse registration response as a string.";
+    DVLOG(1) << "Failed to get registration response body.";
     return NO_RESPONSE_BODY;
   }
 
-  if (source->GetResponseCode() == net::HTTP_OK) {
-    size_t token_pos = response.find(kTokenPrefix);
-    if (token_pos != std::string::npos) {
-      *token = response.substr(token_pos + arraysize(kTokenPrefix) - 1);
-      return SUCCESS;
-    }
-  }
-
-  // If we are able to parse a meaningful known error, let's do so. Some errors
-  // will have HTTP_BAD_REQUEST, some will have HTTP_OK response code.
+  // If we are able to parse a meaningful known error, let's do so. Note that
+  // some errors will have HTTP_OK response code!
   size_t error_pos = response.find(kErrorPrefix);
   if (error_pos != std::string::npos) {
     std::string error = response.substr(
         error_pos + arraysize(kErrorPrefix) - 1);
+    DVLOG(1) << "Registration response error message: " << error;
     return GetStatusFromError(error);
   }
 
   // If we cannot tell what the error is, but at least we know response code was
   // not OK.
   if (source->GetResponseCode() != net::HTTP_OK) {
-    DLOG(ERROR) << "URL fetching HTTP response code is not OK. It is "
-                << source->GetResponseCode();
+    DVLOG(1) << "Registration HTTP response code not OK: "
+             << source->GetResponseCode();
     return HTTP_NOT_OK;
   }
 
-  return UNKNOWN_ERROR;
+  size_t token_pos = response.find(kTokenPrefix);
+  if (token_pos != std::string::npos) {
+    *token = response.substr(token_pos + arraysize(kTokenPrefix) - 1);
+    return SUCCESS;
+  }
+
+  return RESPONSE_PARSING_FAILED;
 }
 
 void RegistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) {
   std::string token;
   Status status = ParseResponse(source, &token);
   recorder_->RecordRegistrationResponse(request_info_.app_id(),
                                         source_to_record_, status);
 
   DCHECK(custom_request_handler_.get());
   custom_request_handler_->ReportUMAs(
@@ skipping 14 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, token);
 }
 
 }  // namespace gcm