Return 500 rather than 503 when response handlers fail. Libprocess was returning 'Service Unavailable' when an HTTP handler Future fails. More appropriate would be an 'Internal Server Error'.
Review: https://reviews.apache.org/r/52053 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/149db236 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/149db236 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/149db236 Branch: refs/heads/master Commit: 149db236c42f77d39c5a7066867a842724a348f1 Parents: 8322b40 Author: Benjamin Mahler <bmah...@apache.org> Authored: Thu Sep 15 18:29:25 2016 -0700 Committer: Benjamin Mahler <bmah...@apache.org> Committed: Wed Sep 21 14:12:32 2016 -0700 ---------------------------------------------------------------------- 3rdparty/libprocess/src/process.cpp | 4 ++-- 3rdparty/libprocess/src/tests/http_tests.cpp | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/149db236/3rdparty/libprocess/src/process.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp index c0d9785..02a1925 100644 --- a/3rdparty/libprocess/src/process.cpp +++ b/3rdparty/libprocess/src/process.cpp @@ -1230,8 +1230,8 @@ bool HttpProxy::process(const Future<Response>& future, const Request& request) // TODO(benh): Consider handling other "states" of future // (discarded, failed, etc) with different HTTP statuses. Response response = future.isFailed() - ? ServiceUnavailable(future.failure()) - : ServiceUnavailable(); + ? InternalServerError(future.failure()) + : InternalServerError("discarded future"); VLOG(1) << "Returning '" << response.status << "'" << " for '" << request.url.path << "'" http://git-wip-us.apache.org/repos/asf/mesos/blob/149db236/3rdparty/libprocess/src/tests/http_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp b/3rdparty/libprocess/src/tests/http_tests.cpp index 24b266d..2538f56 100644 --- a/3rdparty/libprocess/src/tests/http_tests.cpp +++ b/3rdparty/libprocess/src/tests/http_tests.cpp @@ -179,6 +179,15 @@ TEST(HTTPTest, Endpoints) EXPECT_SOME_EQ("chunked", future->headers.get("Transfer-Encoding")); EXPECT_EQ("Hello World\n", future->body); + + // Test that an endpoint handler failure results in a 500. + EXPECT_CALL(*http.process, body(_)) + .WillOnce(Return(Future<http::Response>::failed("failure"))); + + future = http::get(http.process->self(), "body"); + + AWAIT_ASSERT_RESPONSE_STATUS_EQ(http::InternalServerError().status, future); + EXPECT_EQ("failure", future->body); }
