Skip to content

Commit de61315

Browse files
authored
[Networking] Return error on HTTP 4xx and 5xx responses + utils (#64)
This is a bug fix that is also a breaking change. Network requests that technically succeeded in being performed but resulted in HTTP error codes (4xx or 5xx) used to be returned as non-errors (`Result::kOk`). With this change, such responses will be returned with an error status (`Result::kError`) instead to simplify error handling for library users. Additionally, a new helper functions were added to `ResourceResponse` class: * `base::net::ResourceResponse::DataAsString()` * creates a copy of the response data in `std::string` object * `base::net::ResourceResponse::DataAsStringView()` * creates a non-owning view of the response data in `std::string_view` object These should also make it simpler to use response data by library users.
1 parent d65caa8 commit de61315

5 files changed

Lines changed: 45 additions & 12 deletions

File tree

examples/networking/main.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ void LogNetResponse(const base::net::ResourceResponse& response) {
2121
for (const auto& [h, v] : response.headers) {
2222
LOG(INFO) << " " << h << ": " << v;
2323
}
24-
LOG_IF(INFO, !response.data.empty())
25-
<< "Content:\n"
26-
<< std::string{response.data.begin(), response.data.end()};
24+
LOG_IF(INFO, !response.data.empty()) << "Content:\n"
25+
<< response.DataAsStringView();
2726
}
2827

2928
void NetExampleGet() {

src/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ if(LIBBASE_BUILD_MODULE_NET)
164164
base/net/request_cancellation_token.h
165165
base/net/resource_request.cc
166166
base/net/resource_request.h
167+
base/net/resource_response.cc
167168
base/net/resource_response.h
168169
base/net/result.h
169170
base/net/simple_url_loader.cc

src/base/net/impl/net_thread_impl.cc

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ Result CurlCodeToNetResult(CURLcode code) {
2020
}
2121
}
2222

23-
std::tuple<int, std::string, std::map<std::string, std::string>>
23+
std::tuple<int, std::string, std::string, std::map<std::string, std::string>>
2424
GetResponseInfo(CURL* handle, Result result = Result::kOk) {
2525
int code = -1;
26+
std::string scheme;
2627
std::string final_url;
2728
std::map<std::string, std::string> headers;
2829

@@ -31,6 +32,13 @@ GetResponseInfo(CURL* handle, Result result = Result::kOk) {
3132
curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &code);
3233
}
3334

35+
// Protocol scheme
36+
{
37+
char* curl_scheme = nullptr;
38+
curl_easy_getinfo(handle, CURLINFO_SCHEME, &curl_scheme);
39+
scheme = curl_scheme ? curl_scheme : "";
40+
}
41+
3442
// Get the URL of the completed transfer
3543
{
3644
char* final_url_cstr = nullptr;
@@ -46,7 +54,7 @@ GetResponseInfo(CURL* handle, Result result = Result::kOk) {
4654
}
4755
}
4856

49-
return {code, final_url, headers};
57+
return {code, scheme, final_url, headers};
5058
}
5159
} // namespace
5260

@@ -323,8 +331,8 @@ void NetThread::NetThreadImpl::EnqueueDownload_NetThread(
323331
auto response_info = GetResponseInfo(info->handle);
324332
std::move(info->on_response_started)
325333
.Run(std::get<0>(response_info),
326-
std::move(std::get<1>(response_info)),
327-
std::move(std::get<2>(response_info)));
334+
std::move(std::get<2>(response_info)),
335+
std::move(std::get<3>(response_info)));
328336
}
329337

330338
if (info->on_write_data) {
@@ -351,13 +359,18 @@ void NetThread::NetThreadImpl::DownloadFinished_NetThread(CURL* finished_curl,
351359
CHECK(download_iter != active_downloads_.end());
352360

353361
auto& download = download_iter->second;
362+
const auto [code, scheme, final_url, headers] =
363+
GetResponseInfo(finished_curl);
364+
365+
// Override result in case of HTTP status code reporting an error
366+
if (result == Result::kOk && (scheme == "http" || scheme == "https") &&
367+
(code >= 400 && code < 600)) {
368+
result = Result::kError;
369+
}
354370

355371
// Handle advanced path and just send status & do cleanup
356372
if (download.on_response_started) {
357-
auto response_info = GetResponseInfo(finished_curl);
358-
std::move(download.on_response_started)
359-
.Run(std::get<0>(response_info), std::move(std::get<1>(response_info)),
360-
std::move(std::get<2>(response_info)));
373+
std::move(download.on_response_started).Run(code, final_url, headers);
361374
}
362375
if (download.on_finished) {
363376
std::move(download.on_finished).Run(result);
@@ -372,7 +385,7 @@ void NetThread::NetThreadImpl::DownloadFinished_NetThread(CURL* finished_curl,
372385

373386
// Response info
374387
std::tie(download.response.code, download.response.final_url,
375-
download.response.headers) = GetResponseInfo(finished_curl, result);
388+
download.response.headers) = std::tie(code, final_url, headers);
376389

377390
// Get timing data
378391
{

src/base/net/resource_response.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#include "base/net/resource_response.h"
2+
3+
namespace base {
4+
namespace net {
5+
6+
std::string ResourceResponse::DataAsString() const {
7+
return std::string{data.begin(), data.end()};
8+
}
9+
10+
std::string_view ResourceResponse::DataAsStringView() const {
11+
return std::string_view{reinterpret_cast<const char*>(data.data()),
12+
data.size()};
13+
}
14+
15+
} // namespace net
16+
} // namespace base

src/base/net/resource_response.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <cstdint>
66
#include <map>
77
#include <string>
8+
#include <string_view>
89
#include <vector>
910

1011
#include "base/net/result.h"
@@ -24,6 +25,9 @@ struct ResourceResponse {
2425
base::TimeDelta timing_connect = base::Seconds(-1);
2526
base::TimeDelta timing_start_transfer = base::Seconds(-1);
2627
base::TimeDelta timing_total = base::Seconds(-1);
28+
29+
std::string DataAsString() const;
30+
std::string_view DataAsStringView() const;
2731
};
2832

2933
} // namespace net

0 commit comments

Comments
 (0)