AU: Separate error codes for different OmahaRequestAction failures.
BUG=8651
TEST=unit tests, tested an update on the device through dev server
Change-Id: Ic590906be269fe371702bfbe282cddc197ab01fc
Review URL: http://codereview.chromium.org/4432002
diff --git a/action_processor.h b/action_processor.h
index 9bc4ef2..2618c39 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -38,6 +38,12 @@
kActionCodeDownloadPayloadVerificationError = 12,
kActionCodeDownloadAppliedUpdateVerificationError = 13,
kActionCodeDownloadWriteError = 14,
+ kActionCodeOmahaRequestEmptyResponseError = 200,
+ kActionCodeOmahaRequestXMLParseError = 201,
+ kActionCodeOmahaRequestNoUpdateCheckNode = 202,
+ kActionCodeOmahaRequestNoUpdateCheckStatus = 203,
+ kActionCodeOmahaRequestBadUpdateCheckStatus = 204,
+ kActionCodeOmahaRequestHTTPResponseBase = 2000, // + HTTP response code
};
class AbstractAction;
diff --git a/mock_http_fetcher.cc b/mock_http_fetcher.cc
index 2e11865..eec550f 100644
--- a/mock_http_fetcher.cc
+++ b/mock_http_fetcher.cc
@@ -17,7 +17,11 @@
}
void MockHttpFetcher::BeginTransfer(const std::string& url) {
- http_response_code_ = 0;
+ if (fail_transfer_ || data_.empty()) {
+ // No data to send, just notify of completion..
+ SignalTransferComplete();
+ return;
+ }
if (sent_size_ < data_.size())
SendData(true);
}
@@ -26,6 +30,11 @@
// and it needs to be deleted by the caller. If timeout_source_ is NULL
// when this function is called, this function will always return true.
bool MockHttpFetcher::SendData(bool skip_delivery) {
+ if (fail_transfer_) {
+ SignalTransferComplete();
+ return timeout_source_;
+ }
+
CHECK_LT(sent_size_, data_.size());
if (!skip_delivery) {
const size_t chunk_size = min(kMockHttpFetcherChunkSize,
@@ -36,8 +45,7 @@
CHECK_LE(sent_size_, data_.size());
if (sent_size_ == data_.size()) {
// We've sent all the data. Notify of success.
- http_response_code_ = 200;
- delegate_->TransferComplete(this, true);
+ SignalTransferComplete();
}
}
@@ -101,4 +109,18 @@
}
}
+void MockHttpFetcher::FailTransfer(int http_response_code) {
+ fail_transfer_ = true;
+ http_response_code_ = http_response_code;
+}
+
+void MockHttpFetcher::SignalTransferComplete() {
+ // If the transfer has been failed, the HTTP response code should be set
+ // already.
+ if (!fail_transfer_) {
+ http_response_code_ = 200;
+ }
+ delegate_->TransferComplete(this, !fail_transfer_);
+}
+
} // namespace chromeos_update_engine
diff --git a/mock_http_fetcher.h b/mock_http_fetcher.h
index f0830f0..dfe61a7 100644
--- a/mock_http_fetcher.h
+++ b/mock_http_fetcher.h
@@ -27,7 +27,11 @@
// The data passed in here is copied and then passed to the delegate after
// the transfer begins.
MockHttpFetcher(const char* data, size_t size)
- : sent_size_(0), timeout_source_(NULL), timout_tag_(0), paused_(false) {
+ : sent_size_(0),
+ timeout_source_(NULL),
+ timout_tag_(0),
+ paused_(false),
+ fail_transfer_(false) {
data_.insert(data_.end(), data, data + size);
}
@@ -55,7 +59,7 @@
virtual void Unpause();
// Fail the transfer. This simulates a network failure.
- void FailTransfer();
+ void FailTransfer(int http_response_code);
const std::vector<char>& post_data() const {
return post_data_;
@@ -76,6 +80,10 @@
return reinterpret_cast<MockHttpFetcher*>(data)->TimeoutCallback();
}
+ // Sets the HTTP response code and signals to the delegate that the transfer
+ // is complete.
+ void SignalTransferComplete();
+
// A full copy of the data we'll return to the delegate
std::vector<char> data_;
@@ -92,6 +100,9 @@
// True iff the fetcher is paused.
bool paused_;
+ // Set to true if the transfer should fail.
+ bool fail_transfer_;
+
DISALLOW_COPY_AND_ASSIGN(MockHttpFetcher);
};
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index ecee25e..5166315 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -329,6 +329,13 @@
if (!successful) {
LOG(ERROR) << "Omaha request network transfer failed.";
+ int code = GetHTTPResponseCode();
+ // Makes sure we send sane error values.
+ if (code < 0 || code >= 1000) {
+ code = 999;
+ }
+ completer.set_code(static_cast<ActionExitCode>(
+ kActionCodeOmahaRequestHTTPResponseBase + code));
return;
}
if (!HasOutputPipe()) {
@@ -343,6 +350,9 @@
xmlParseMemory(&response_buffer_[0], response_buffer_.size()));
if (!doc.get()) {
LOG(ERROR) << "Omaha response not valid XML";
+ completer.set_code(response_buffer_.empty() ?
+ kActionCodeOmahaRequestEmptyResponseError :
+ kActionCodeOmahaRequestXMLParseError);
return;
}
@@ -366,6 +376,7 @@
ConstXMLStr(kNamespace),
ConstXMLStr(kNsUrl)));
if (!xpath_nodeset.get()) {
+ completer.set_code(kActionCodeOmahaRequestNoUpdateCheckNode);
return;
}
xmlNodeSet* nodeset = xpath_nodeset->nodesetval;
@@ -376,6 +387,7 @@
// get status
if (!xmlHasProp(updatecheck_node, ConstXMLStr("status"))) {
LOG(ERROR) << "Response missing status";
+ completer.set_code(kActionCodeOmahaRequestNoUpdateCheckStatus);
return;
}
@@ -393,6 +405,7 @@
if (status != "ok") {
LOG(ERROR) << "Unknown status: " << status;
+ completer.set_code(kActionCodeOmahaRequestBadUpdateCheckStatus);
return;
}
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index f822357..2501d2d 100755
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -145,19 +145,24 @@
};
// Returns true iff an output response was obtained from the
-// OmahaRequestAction. |prefs| may be NULL, in which case a local
-// PrefsMock is used. out_response may be NULL. out_post_data may be
-// null; if non-null, the post-data received by the mock HttpFetcher
-// is returned.
+// OmahaRequestAction. |prefs| may be NULL, in which case a local PrefsMock is
+// used. out_response may be NULL. If |fail_http_response_code| is
+// non-negative, the transfer will fail with that code. out_post_data may be
+// null; if non-null, the post-data received by the mock HttpFetcher is
+// returned.
bool TestUpdateCheck(PrefsInterface* prefs,
const OmahaRequestParams& params,
const string& http_response,
+ int fail_http_response_code,
ActionExitCode expected_code,
OmahaResponse* out_response,
vector<char>* out_post_data) {
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
MockHttpFetcher* fetcher = new MockHttpFetcher(http_response.data(),
http_response.size());
+ if (fail_http_response_code >= 0) {
+ fetcher->FailTransfer(fail_http_response_code);
+ }
PrefsMock local_prefs;
OmahaRequestAction action(prefs ? prefs : &local_prefs,
params,
@@ -216,6 +221,7 @@
TestUpdateCheck(NULL, // prefs
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ -1,
kActionCodeSuccess,
&response,
NULL));
@@ -236,6 +242,7 @@
"false", // needs admin
"123", // size
"20101020"), // deadline
+ -1,
kActionCodeSuccess,
&response,
NULL));
@@ -278,7 +285,21 @@
TestUpdateCheck(NULL, // prefs
kDefaultTestParams,
"invalid xml>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestXMLParseError,
+ &response,
+ NULL));
+ EXPECT_FALSE(response.update_exists);
+}
+
+TEST(OmahaRequestActionTest, EmptyResponseTest) {
+ OmahaResponse response;
+ ASSERT_FALSE(
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
+ "",
+ -1,
+ kActionCodeOmahaRequestEmptyResponseError,
&response,
NULL));
EXPECT_FALSE(response.update_exists);
@@ -293,7 +314,8 @@
"xmlns=\"http://www.google.com/update2/response\" protocol=\"2.0\"><app "
"appid=\"foo\" status=\"ok\"><ping "
"status=\"ok\"/><updatecheck/></app></gupdate>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestNoUpdateCheckStatus,
&response,
NULL));
EXPECT_FALSE(response.update_exists);
@@ -308,7 +330,8 @@
"xmlns=\"http://www.google.com/update2/response\" protocol=\"2.0\"><app "
"appid=\"foo\" status=\"ok\"><ping "
"status=\"ok\"/><updatecheck status=\"foo\"/></app></gupdate>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestBadUpdateCheckStatus,
&response,
NULL));
EXPECT_FALSE(response.update_exists);
@@ -323,7 +346,8 @@
"xmlns=\"http://www.google.com/update2/response\" protocol=\"2.0\"><app "
"appid=\"foo\" status=\"ok\"><ping "
"status=\"ok\"/></app></gupdate>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestNoUpdateCheckNode,
&response,
NULL));
EXPECT_FALSE(response.update_exists);
@@ -347,6 +371,7 @@
"sha256=\"HASH1234=\" needsadmin=\"true\" "
"size=\"123\" "
"status=\"ok\"/></app></gupdate>",
+ -1,
kActionCodeSuccess,
&response,
NULL));
@@ -426,7 +451,8 @@
TestUpdateCheck(NULL, // prefs
params,
"invalid xml>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestXMLParseError,
&response,
&post_data));
// convert post_data to string
@@ -455,6 +481,7 @@
"false", // needs admin
"123", // size
"<20110101"), // deadline
+ -1,
kActionCodeSuccess,
&response,
NULL));
@@ -479,6 +506,7 @@
// overflows int32:
"123123123123123", // size
"deadline"),
+ -1,
kActionCodeSuccess,
&response,
NULL));
@@ -491,7 +519,8 @@
ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
kDefaultTestParams,
"invalid xml>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestXMLParseError,
NULL, // response
&post_data));
// convert post_data to string
@@ -600,7 +629,8 @@
ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
params,
"invalid xml>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestXMLParseError,
NULL,
&post_data));
// convert post_data to string
@@ -646,6 +676,7 @@
TestUpdateCheck(&prefs,
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ -1,
kActionCodeSuccess,
NULL,
&post_data));
@@ -667,6 +698,7 @@
TestUpdateCheck(&prefs,
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ -1,
kActionCodeSuccess,
NULL,
&post_data));
@@ -688,6 +720,7 @@
TestUpdateCheck(&prefs,
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ -1,
kActionCodeSuccess,
NULL,
&post_data));
@@ -710,6 +743,7 @@
TestUpdateCheck(&prefs,
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ -1,
kActionCodeSuccess,
NULL,
&post_data));
@@ -738,6 +772,7 @@
"protocol=\"2.0\"><daystart elapsed_seconds=\"100\"/>"
"<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
"<updatecheck status=\"noupdate\"/></app></gupdate>",
+ -1,
kActionCodeSuccess,
NULL,
&post_data));
@@ -769,6 +804,7 @@
"protocol=\"2.0\"><daystart elapsed_seconds=\"200\"/>"
"<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
"<updatecheck status=\"noupdate\"/></app></gupdate>",
+ -1,
kActionCodeSuccess,
NULL,
NULL));
@@ -786,6 +822,7 @@
"protocol=\"2.0\"><daystart blah=\"200\"/>"
"<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
"<updatecheck status=\"noupdate\"/></app></gupdate>",
+ -1,
kActionCodeSuccess,
NULL,
NULL));
@@ -803,6 +840,7 @@
"protocol=\"2.0\"><daystart elapsed_seconds=\"x\"/>"
"<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
"<updatecheck status=\"noupdate\"/></app></gupdate>",
+ -1,
kActionCodeSuccess,
NULL,
NULL));
@@ -813,7 +851,8 @@
ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
kDefaultTestParams,
"invalid xml>",
- kActionCodeError,
+ -1,
+ kActionCodeOmahaRequestXMLParseError,
NULL, // response
&post_data));
// convert post_data to string
@@ -822,4 +861,32 @@
EXPECT_EQ(post_str.find("userid="), string::npos);
}
+TEST(OmahaRequestActionTest, NetworkFailureTest) {
+ OmahaResponse response;
+ ASSERT_FALSE(
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
+ "",
+ 501,
+ static_cast<ActionExitCode>(
+ kActionCodeOmahaRequestHTTPResponseBase + 501),
+ &response,
+ NULL));
+ EXPECT_FALSE(response.update_exists);
+}
+
+TEST(OmahaRequestActionTest, NetworkFailureBadHTTPCodeTest) {
+ OmahaResponse response;
+ ASSERT_FALSE(
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
+ "",
+ 1500,
+ static_cast<ActionExitCode>(
+ kActionCodeOmahaRequestHTTPResponseBase + 999),
+ &response,
+ NULL));
+ EXPECT_FALSE(response.update_exists);
+}
+
} // namespace chromeos_update_engine