Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(116)

Unified Diff: google_apis/gcm/engine/unregistration_request.cc

Issue 2434243002: GCM Engine: Split up reg/unreg UNKNOWN_ERROR to improve metrics (Closed)
Patch Set: mid-cycle -> mid-beta Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: google_apis/gcm/engine/unregistration_request.cc
diff --git a/google_apis/gcm/engine/unregistration_request.cc b/google_apis/gcm/engine/unregistration_request.cc
index 15c8d883fa39dc806c2475555f849dd26c8b89fa..b7a07e4e6a60648a105dd01c941b351d47f63cf6 100644
--- a/google_apis/gcm/engine/unregistration_request.cc
+++ b/google_apis/gcm/engine/unregistration_request.cc
@@ -36,6 +36,24 @@ 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 kDeviceRegistrationError[] = "PHONE_REGISTRATION_ERROR";
+
+// 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(kDeviceRegistrationError) != 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) {
@@ -49,6 +67,7 @@ bool ShouldRetryWithStatus(UnregistrationRequest::Status status) {
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;
@@ -159,14 +178,29 @@ void UnregistrationRequest::BuildRequestBody(std::string* 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)
@@ -175,7 +209,7 @@ UnregistrationRequest::Status UnregistrationRequest::ParseResponse(
}
DCHECK(custom_request_handler_.get());
- return custom_request_handler_->ParseResponse(source);
+ return custom_request_handler_->ParseResponse(response);
}
void UnregistrationRequest::RetryWithBackoff() {
« no previous file with comments | « google_apis/gcm/engine/unregistration_request.h ('k') | google_apis/gcm/engine/unregistration_request_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698