Skip to content

Commit

Permalink
LibWeb+LibRequests+RequestServer: Report network error on request finish
Browse files Browse the repository at this point in the history
This allows us to bubble up network errors to API consumers after
finishing a request.
  • Loading branch information
rmg-x committed Oct 9, 2024
1 parent 7faebb2 commit cddf8da
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 19 deletions.
3 changes: 2 additions & 1 deletion Ladybird/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ endfunction()

add_executable(headless-browser
${LADYBIRD_SOURCE_DIR}/Userland/Utilities/headless-browser.cpp
${LADYBIRD_SOURCES})
${LADYBIRD_SOURCES}
../Userland/Libraries/LibRequests/NetworkErrorEnum.h)

target_include_directories(headless-browser PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
target_include_directories(headless-browser PRIVATE ${LADYBIRD_SOURCE_DIR}/Userland/)
Expand Down
1 change: 1 addition & 0 deletions Userland/Libraries/LibRequests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ set(SOURCES
Request.cpp
RequestClient.cpp
WebSocket.cpp
NetworkErrorEnum.h
)

set(GENERATED_SOURCES
Expand Down
22 changes: 22 additions & 0 deletions Userland/Libraries/LibRequests/NetworkErrorEnum.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2024, the Ladybird developers.
*
* SPDX-License-Identifier: BSD-2-Clause
*/

#pragma once

namespace Requests {

enum class NetworkError {
UnableToResolveProxy,
UnableToResolveHost,
UnableToConnect,
TimeoutReached,
TooManyRedirects,
SSLHandshakeFailed,
SSLVerificationFailed,
Unknown
};

}
12 changes: 7 additions & 5 deletions Userland/Libraries/LibRequests/Request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,14 @@ void Request::set_buffered_request_finished_callback(BufferedRequestFinished on_
m_internal_buffered_data->response_code = move(response_code);
};

on_finish = [this, on_buffered_request_finished = move(on_buffered_request_finished)](auto success, auto total_size) {
on_finish = [this, on_buffered_request_finished = move(on_buffered_request_finished)](auto success, auto total_size, auto network_error) {
auto output_buffer = ByteBuffer::create_uninitialized(m_internal_buffered_data->payload_stream.used_buffer_size()).release_value_but_fixme_should_propagate_errors();
m_internal_buffered_data->payload_stream.read_until_filled(output_buffer).release_value_but_fixme_should_propagate_errors();

on_buffered_request_finished(
success,
total_size,
network_error,
m_internal_buffered_data->response_headers,
m_internal_buffered_data->response_code,
output_buffer);
Expand All @@ -81,10 +82,10 @@ void Request::set_unbuffered_request_callbacks(HeadersReceived on_headers_receiv
set_up_internal_stream_data(move(on_data_received));
}

void Request::did_finish(Badge<RequestClient>, bool success, u64 total_size)
void Request::did_finish(Badge<RequestClient>, bool success, u64 total_size, Optional<NetworkError> const& network_error)
{
if (on_finish)
on_finish(success, total_size);
on_finish(success, total_size, network_error);
}

void Request::did_receive_headers(Badge<RequestClient>, HTTP::HeaderMap const& response_headers, Optional<u32> response_code)
Expand Down Expand Up @@ -113,17 +114,18 @@ void Request::set_up_internal_stream_data(DataReceived on_data_available)
m_internal_stream_data->read_stream = MUST(Core::File::adopt_fd(fd(), Core::File::OpenMode::Read));

auto user_on_finish = move(on_finish);
on_finish = [this](auto success, auto total_size) {
on_finish = [this](auto success, auto total_size, auto network_error) {
m_internal_stream_data->success = success;
m_internal_stream_data->total_size = total_size;
m_internal_stream_data->network_error = network_error;
m_internal_stream_data->request_done = true;
m_internal_stream_data->on_finish();
};

m_internal_stream_data->on_finish = [this, user_on_finish = move(user_on_finish)]() {
if (!m_internal_stream_data->user_finish_called && m_internal_stream_data->read_stream->is_eof()) {
m_internal_stream_data->user_finish_called = true;
user_on_finish(m_internal_stream_data->success, m_internal_stream_data->total_size);
user_on_finish(m_internal_stream_data->success, m_internal_stream_data->total_size, m_internal_stream_data->network_error);
}
};

Expand Down
9 changes: 6 additions & 3 deletions Userland/Libraries/LibRequests/Request.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#pragma once

#include "NetworkErrorEnum.h"

#include <AK/Badge.h>
#include <AK/ByteBuffer.h>
#include <AK/ByteString.h>
Expand Down Expand Up @@ -37,23 +39,23 @@ class Request : public RefCounted<Request> {
int fd() const { return m_fd; }
bool stop();

using BufferedRequestFinished = Function<void(bool success, u64 total_size, HTTP::HeaderMap const& response_headers, Optional<u32> response_code, ReadonlyBytes payload)>;
using BufferedRequestFinished = Function<void(bool success, u64 total_size, Optional<NetworkError> const& network_error, HTTP::HeaderMap const& response_headers, Optional<u32> response_code, ReadonlyBytes payload)>;

// Configure the request such that the entirety of the response data is buffered. The callback receives that data and
// the response headers all at once. Using this method is mutually exclusive with `set_unbuffered_data_received_callback`.
void set_buffered_request_finished_callback(BufferedRequestFinished);

using HeadersReceived = Function<void(HTTP::HeaderMap const& response_headers, Optional<u32> response_code)>;
using DataReceived = Function<void(ReadonlyBytes data)>;
using RequestFinished = Function<void(bool success, u64 total_size)>;
using RequestFinished = Function<void(bool success, u64 total_size, Optional<NetworkError> network_error)>;

// Configure the request such that the response data is provided unbuffered as it is received. Using this method is
// mutually exclusive with `set_buffered_request_finished_callback`.
void set_unbuffered_request_callbacks(HeadersReceived, DataReceived, RequestFinished);

Function<CertificateAndKey()> on_certificate_requested;

void did_finish(Badge<RequestClient>, bool success, u64 total_size);
void did_finish(Badge<RequestClient>, bool success, u64 total_size, Optional<NetworkError> const& network_error);
void did_receive_headers(Badge<RequestClient>, HTTP::HeaderMap const& response_headers, Optional<u32> response_code);
void did_request_certificates(Badge<RequestClient>);

Expand Down Expand Up @@ -93,6 +95,7 @@ class Request : public RefCounted<Request> {
RefPtr<Core::Notifier> read_notifier;
bool success;
u32 total_size { 0 };
Optional<NetworkError> network_error;
bool request_done { false };
Function<void()> on_finish {};
bool user_finish_called { false };
Expand Down
4 changes: 2 additions & 2 deletions Userland/Libraries/LibRequests/RequestClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ bool RequestClient::set_certificate(Badge<Request>, Request& request, ByteString
return IPCProxy::set_certificate(request.id(), move(certificate), move(key));
}

void RequestClient::request_finished(i32 request_id, bool success, u64 total_size)
void RequestClient::request_finished(i32 request_id, bool success, u64 total_size, Optional<NetworkError> const& network_error)
{
RefPtr<Request> request;
if ((request = m_requests.get(request_id).value_or(nullptr))) {
request->did_finish({}, success, total_size);
request->did_finish({}, success, total_size, network_error);
}
m_requests.remove(request_id);
}
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibRequests/RequestClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class RequestClient final
virtual void die() override;

virtual void request_started(i32, IPC::File const&) override;
virtual void request_finished(i32, bool, u64) override;
virtual void request_finished(i32, bool, u64, Optional<NetworkError> const&) override;
virtual void certificate_requested(i32) override;
virtual void headers_became_available(i32, HTTP::HeaderMap const&, Optional<u32> const&) override;

Expand Down
34 changes: 30 additions & 4 deletions Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,28 @@ static void log_filtered_request(LoadRequest const& request)
dbgln("ResourceLoader: Filtered request to: \"{}\"", url_for_logging);
}

static StringView network_error_to_string_view(Requests::NetworkError const& network_error)
{
switch (network_error) {
case Requests::NetworkError::UnableToResolveProxy:
return "Unable to resolve proxy"sv;
case Requests::NetworkError::UnableToResolveHost:
return "Unable to resolve host"sv;
case Requests::NetworkError::UnableToConnect:
return "Unable to connect"sv;
case Requests::NetworkError::TimeoutReached:
return "Timeout reached"sv;
case Requests::NetworkError::TooManyRedirects:
return "Too many redirects"sv;
case Requests::NetworkError::SSLHandshakeFailed:
return "SSL handshake failed"sv;
case Requests::NetworkError::SSLVerificationFailed:
return "SSL verification failed"sv;
default:
return "An unexpected network error occurred"sv;
}
}

static bool should_block_request(LoadRequest const& request)
{
auto const& url = request.url();
Expand Down Expand Up @@ -397,16 +419,20 @@ void ResourceLoader::load(LoadRequest& request, SuccessCallback success_callback
timer->start();
}

auto on_buffered_request_finished = [this, success_callback = move(success_callback), error_callback = move(error_callback), request, &protocol_request = *protocol_request](bool success, auto, auto& response_headers, auto status_code, ReadonlyBytes payload) mutable {
auto on_buffered_request_finished = [this, success_callback = move(success_callback), error_callback = move(error_callback), request, &protocol_request = *protocol_request](bool success, auto, auto const& network_error, auto& response_headers, auto status_code, ReadonlyBytes payload) mutable {
handle_network_response_headers(request, response_headers);
finish_network_request(protocol_request);

if (!success || (status_code.has_value() && *status_code >= 400 && *status_code <= 599 && (payload.is_empty() || !request.is_main_resource()))) {
StringBuilder error_builder;
if (status_code.has_value())
error_builder.appendff("Load failed: {}", *status_code);
if (network_error.has_value())
error_builder.appendff("{}", network_error_to_string_view(*network_error));
else
error_builder.append("Load failed"sv);

if (status_code.has_value())
error_builder.appendff(" (status code: {})", *status_code);

log_failure(request, error_builder.string_view());
if (error_callback)
error_callback(error_builder.to_byte_string(), status_code, payload, response_headers);
Expand Down Expand Up @@ -460,7 +486,7 @@ void ResourceLoader::load_unbuffered(LoadRequest& request, OnHeadersReceived on_
on_data_received(data);
};

auto protocol_complete = [this, on_complete = move(on_complete), request, &protocol_request = *protocol_request](bool success, u64) {
auto protocol_complete = [this, on_complete = move(on_complete), request, &protocol_request = *protocol_request](bool success, u64, Optional<Requests::NetworkError> const&) {
finish_network_request(protocol_request);

if (success) {
Expand Down
40 changes: 38 additions & 2 deletions Userland/Services/RequestServer/ConnectionFromClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <LibCore/EventLoop.h>
#include <LibCore/Proxy.h>
#include <LibCore/Socket.h>
#include <LibRequests/NetworkErrorEnum.h>
#include <LibWebSocket/ConnectionInfo.h>
#include <LibWebSocket/Message.h>
#include <RequestServer/ConnectionFromClient.h>
Expand Down Expand Up @@ -243,7 +244,7 @@ void ConnectionFromClient::start_request(i32 request_id, ByteString const& metho
{
if (!url.is_valid()) {
dbgln("StartRequest: Invalid URL requested: '{}'", url);
async_request_finished(request_id, false, 0);
async_request_finished(request_id, false, 0, {});
return;
}

Expand Down Expand Up @@ -320,6 +321,28 @@ void ConnectionFromClient::start_request(i32 request_id, ByteString const& metho
m_active_requests.set(request_id, move(request));
}

static Requests::NetworkError map_curl_code_to_network_error(CURLcode const& code)
{
switch (code) {
case CURLE_COULDNT_RESOLVE_HOST:
return Requests::NetworkError::UnableToResolveHost;
case CURLE_COULDNT_RESOLVE_PROXY:
return Requests::NetworkError::UnableToResolveProxy;
case CURLE_COULDNT_CONNECT:
return Requests::NetworkError::UnableToConnect;
case CURLE_OPERATION_TIMEDOUT:
return Requests::NetworkError::TimeoutReached;
case CURLE_TOO_MANY_REDIRECTS:
return Requests::NetworkError::TooManyRedirects;
case CURLE_SSL_CONNECT_ERROR:
return Requests::NetworkError::SSLHandshakeFailed;
case CURLE_PEER_FAILED_VERIFICATION:
return Requests::NetworkError::SSLVerificationFailed;
default:
return Requests::NetworkError::Unknown;
}
}

void ConnectionFromClient::check_active_requests()
{
int msgs_in_queue = 0;
Expand All @@ -332,7 +355,20 @@ void ConnectionFromClient::check_active_requests()
VERIFY(result == CURLE_OK);
request->flush_headers_if_needed();

async_request_finished(request->request_id, msg->data.result == CURLE_OK, request->downloaded_so_far);
auto result_code = msg->data.result;

Optional<Requests::NetworkError> network_error;
bool const request_was_successful = result_code == CURLE_OK;
if (!request_was_successful) {
network_error = map_curl_code_to_network_error(result_code);

if (network_error.has_value() && network_error.value() == Requests::NetworkError::Unknown) {
char const* curl_error_message = curl_easy_strerror(result_code);
dbgln("ConnectionFromClient: Unable to map error ({}), message: \"\033[31;1m{}\033[0m\"", static_cast<int>(result_code), curl_error_message);
}
}

async_request_finished(request->request_id, request_was_successful, request->downloaded_so_far, network_error);

m_active_requests.remove(request->request_id);
}
Expand Down
3 changes: 2 additions & 1 deletion Userland/Services/RequestServer/RequestClient.ipc
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
#include <LibHTTP/HeaderMap.h>
#include <LibRequests/NetworkErrorEnum.h>
#include <LibURL/URL.h>

endpoint RequestClient
{
request_started(i32 request_id, IPC::File fd) =|
request_finished(i32 request_id, bool success, u64 total_size) =|
request_finished(i32 request_id, bool success, u64 total_size, Optional<Requests::NetworkError> network_error) =|
headers_became_available(i32 request_id, HTTP::HeaderMap response_headers, Optional<u32> status_code) =|

// Websocket API
Expand Down

0 comments on commit cddf8da

Please sign in to comment.