Skip to content

Commit 610c8cb

Browse files
authored
Fix exception propaation for client errors (#754)
We change the cross-thread exception propagation given the asynchronous client implementation. The previous implementation caused double-wrapping of exceptions which works with the std::shared_future<...> implementation, but not with boost::shared_future<...> -- since Boost.Thread still uses Boost.Exception for the APIs. While we're here we make the exception propagation more explicit using boost::rethrow_exception(...), so we can unwrap the exception that's transported via an exception_ptr. Fixes #749.
1 parent fa76515 commit 610c8cb

File tree

3 files changed

+70
-30
lines changed

3 files changed

+70
-30
lines changed

boost/network/protocol/http/client/connection/async_normal.hpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,9 @@ struct http_async_connection
160160
request_strand_(resolver.get_io_service()),
161161
delegate_(std::move(delegate)) {}
162162

163-
// This is the main entry point for the connection/request pipeline.
164-
// We're
165-
// overriding async_connection_base<...>::start(...) here which is
166-
// called
167-
// by the client.
163+
// This is the main entry point for the connection/request pipeline. We're
164+
// overriding async_connection_base<...>::start(...) here which is called by
165+
// the client.
168166
virtual response start(request const& request, string_type const& method,
169167
bool get_body, body_callback_function_type callback,
170168
body_generator_function_type generator) {
@@ -198,13 +196,13 @@ struct http_async_connection
198196
private:
199197
void set_errors(boost::system::error_code const& ec, body_callback_function_type callback) {
200198
boost::system::system_error error(ec);
201-
this->version_promise.set_exception(std::make_exception_ptr(error));
202-
this->status_promise.set_exception(std::make_exception_ptr(error));
203-
this->status_message_promise.set_exception(std::make_exception_ptr(error));
204-
this->headers_promise.set_exception(std::make_exception_ptr(error));
205-
this->source_promise.set_exception(std::make_exception_ptr(error));
206-
this->destination_promise.set_exception(std::make_exception_ptr(error));
207-
this->body_promise.set_exception(std::make_exception_ptr(error));
199+
this->version_promise.set_exception(boost::copy_exception(error));
200+
this->status_promise.set_exception(boost::copy_exception(error));
201+
this->status_message_promise.set_exception(boost::copy_exception(error));
202+
this->headers_promise.set_exception(boost::copy_exception(error));
203+
this->source_promise.set_exception(boost::copy_exception(error));
204+
this->destination_promise.set_exception(boost::copy_exception(error));
205+
this->body_promise.set_exception(boost::copy_exception(error));
208206
if ( callback )
209207
callback( boost::iterator_range<typename std::array<typename char_<Tag>::type, 1024>::const_iterator>(), ec );
210208
this->timer_.cancel();

boost/network/protocol/http/message/async_message.hpp

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@
99
// (See accompanying file LICENSE_1_0.txt or copy at
1010
// http://www.boost.org/LICENSE_1_0.txt)
1111

12-
#include <cstdint>
1312
#include <boost/optional.hpp>
13+
#include <cstdint>
1414

1515
// FIXME move this out to a trait
16-
#include <set>
1716
#include <boost/network/detail/wrapper_base.hpp>
1817
#include <boost/thread/future.hpp>
18+
#include <set>
1919

2020
namespace boost {
2121
namespace network {
@@ -26,12 +26,11 @@ namespace impl {
2626
template <class Tag>
2727
struct ready_wrapper;
2828

29-
} // namespace impl
30-
/* impl */
29+
} // namespace impl
30+
/* impl */
3131

3232
template <class Tag>
3333
struct async_message {
34-
3534
typedef typename string<Tag>::type string_type;
3635
typedef typename headers_container<Tag>::type headers_container_type;
3736
typedef typename headers_container_type::value_type header_type;
@@ -54,63 +53,95 @@ struct async_message {
5453
headers_(other.headers_),
5554
body_(other.body_) {}
5655

57-
string_type const status_message() const { return status_message_.get(); }
56+
string_type status_message() const {
57+
status_message_.wait();
58+
if (status_message_.has_exception())
59+
boost::rethrow_exception(status_message_.get_exception_ptr());
60+
return status_message_.get();
61+
}
5862

5963
void status_message(boost::shared_future<string_type> const& future) const {
6064
status_message_ = future;
6165
}
6266

63-
string_type const version() const { return version_.get(); }
67+
string_type version() const {
68+
version_.wait();
69+
if (version_.has_exception())
70+
boost::rethrow_exception(version_.get_exception_ptr());
71+
return version_.get();
72+
}
6473

6574
void version(boost::shared_future<string_type> const& future) const {
6675
version_ = future;
6776
}
6877

69-
std::uint16_t status() const { return status_.get(); }
78+
std::uint16_t status() const {
79+
status_.wait();
80+
if (status_.has_exception())
81+
boost::rethrow_exception(status_.get_exception_ptr());
82+
return status_.get();
83+
}
7084

7185
void status(boost::shared_future<uint16_t> const& future) const {
7286
status_ = future;
7387
}
7488

75-
string_type const source() const { return source_.get(); }
89+
string_type source() const {
90+
source_.wait();
91+
if (source_.has_exception())
92+
boost::rethrow_exception(source_.get_exception_ptr());
93+
return source_.get();
94+
}
7695

7796
void source(boost::shared_future<string_type> const& future) const {
7897
source_ = future;
7998
}
8099

81-
string_type const destination() const { return destination_.get(); }
100+
string_type destination() const {
101+
destination_.wait();
102+
if (destination_.has_exception())
103+
boost::rethrow_exception(source_.get_exception_ptr());
104+
return destination_.get();
105+
}
82106

83107
void destination(boost::shared_future<string_type> const& future) const {
84108
destination_ = future;
85109
}
86110

87111
headers_container_type const& headers() const {
88112
if (retrieved_headers_) return *retrieved_headers_;
113+
if (headers_.has_exception())
114+
boost::rethrow_exception(headers_.get_exception_ptr());
89115
headers_container_type raw_headers = headers_.get();
90116
raw_headers.insert(added_headers.begin(), added_headers.end());
91-
for (string_type const & key : removed_headers) {
117+
for (string_type const& key : removed_headers) {
92118
raw_headers.erase(key);
93119
}
94120
retrieved_headers_ = raw_headers;
95121
return *retrieved_headers_;
96122
}
97123

98-
void headers(boost::shared_future<headers_container_type> const& future)
99-
const {
124+
void headers(
125+
boost::shared_future<headers_container_type> const& future) const {
100126
headers_ = future;
101127
}
102128

103-
void add_header(typename headers_container_type::value_type const& pair_)
104-
const {
129+
void add_header(
130+
typename headers_container_type::value_type const& pair_) const {
105131
added_headers.insert(added_headers.end(), pair_);
106132
}
107133

108-
void remove_header(typename headers_container_type::key_type const& key_)
109-
const {
134+
void remove_header(
135+
typename headers_container_type::key_type const& key_) const {
110136
removed_headers.insert(key_);
111137
}
112138

113-
string_type const body() const { return body_.get(); }
139+
string_type body() const {
140+
body_.wait();
141+
if (body_.has_exception())
142+
boost::rethrow_exception(body_.get_exception_ptr());
143+
return body_.get();
144+
}
114145

115146
void body(boost::shared_future<string_type> const& future) const {
116147
body_ = future;

libs/network/test/http/client_get_test.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Copyright 2010 Dean Michael Berris.
2+
// Copyright 2017 Google, Inc.
23
// Distributed under the Boost Software License, Version 1.0.
34
// (See accompanying file LICENSE_1_0.txt or copy at
45
// http://www.boost.org/LICENSE_1_0.txt)
@@ -71,3 +72,13 @@ TYPED_TEST(HTTPClientTest, TemporaryClientObjectTest) {
7172
EXPECT_TRUE(response.status() == 200u ||
7273
(response.status() >= 300 && response.status() < 400));
7374
}
75+
76+
TYPED_TEST(HTTPClientTest, PropagatesResolutionErrorsTest) {
77+
using client = TypeParam;
78+
typename client::request request("http://malformed.google.comq");
79+
typename client::response response;
80+
typename client::options options;
81+
typename client::string_type response_body;
82+
ASSERT_NO_THROW(response = client(options).get(request));
83+
EXPECT_THROW(response_body = body(response), std::exception);
84+
}

0 commit comments

Comments
 (0)