From d24970b2070db2257918997ccb6b8777d21165bd Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Thu, 8 Oct 2020 18:47:04 -0500 Subject: [PATCH 1/6] gsocketclient: fix docs typo --- gio/gsocketclient.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index ca01b68f4..f003aef6b 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -828,7 +828,7 @@ g_socket_client_class_init (GSocketClientClass *class) * multiple times (or not at all) for a given connectable (in * particular, if @client ends up attempting to connect to more than * one address). However, if @client emits the #GSocketClient::event - * signal at all for a given connectable, that it will always emit + * signal at all for a given connectable, then it will always emit * it with %G_SOCKET_CLIENT_COMPLETE when it is done. * * Note that there may be additional #GSocketClientEvent values in From d971ac7b21a1b42e098c2fde3148ca4a6e75626a Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Thu, 8 Oct 2020 13:42:14 -0500 Subject: [PATCH 2/6] gsocketclient: fix whitespace error --- gio/gsocketclient.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index f003aef6b..0a9e9df82 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -945,7 +945,7 @@ g_socket_client_class_init (GSocketClientClass *class) static void g_socket_client_emit_event (GSocketClient *client, - GSocketClientEvent event, + GSocketClientEvent event, GSocketConnectable *connectable, GIOStream *connection) { From 290d5722beb9405ffbb1a6262e94bf6de4db6f5e Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Thu, 8 Oct 2020 19:08:34 -0500 Subject: [PATCH 3/6] gsocketclient: document Happy Eyeballs This isn't an API guarantee, but it's a potentially-surprising behavior difference between the sync and async functions that is good to know about, especially because our sync and async functions are normally identical. --- gio/gsocketclient.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index 0a9e9df82..8a663c3de 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -1936,6 +1936,15 @@ g_socket_client_enumerator_callback (GObject *object, * * This is the asynchronous version of g_socket_client_connect(). * + * You may wish to prefer the asynchronous version even in synchronous + * command line programs because, since 2.60, it implements + * [RFC 8305](https://tools.ietf.org/html/rfc8305) "Happy Eyeballs" + * recommendations to work around long connection timeouts in networks + * where IPv6 is broken by performing an IPv4 connection simultaneously + * without waiting for IPv6 to time out, which is not supported by the + * synchronous call. (This is not an API guarantee, and may change in + * the future.) + * * When the operation is finished @callback will be * called. You can then call g_socket_client_connect_finish() to get * the result of the operation. From f0a7b147806e852e2090eeda6e4e38f7d3f52b52 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Tue, 6 Oct 2020 15:39:45 -0500 Subject: [PATCH 4/6] gsocketclient: emit RESOLVING/RESOLVED events only once GSocketAddressEnumerator encapsulates the details of how DNS happens, so we don't have to think about it. But we may have taken encapsulation a bit too far, here. Usually, we resolve a domain name to a list of IPv4 and IPv6 addresses. Then we go through each address in the list and try to connect to it. Name resolution happens exactly once, at the start. It doesn't happen each time we enumerate the enumerator. In theory, it *could*, because we've designed these APIs to be agnostic of underlying implementation details like DNS and network protocols. But in practice, we know that's not really what's happening. It's weird to say that we are RESOLVING what we know to be the same name multiple times. Behind the scenes, we're not doing that. This also fixes #1994, where enumeration can end with a RESOLVING event, even though this is supposed to be the first event rather than the last. I thought this would be hard to fix, even requiring new public API in GSocketAddressEnumerator to peek ahead to see if the next enumeration is going to return NULL. Then I decided we should just fake it: always emit both RESOLVING and RESOLVED at the same time right after each enumeration. Finally, I realized we can emit them at the correct time if we simply assume resolving only happens the first time. This seems like the most elegant of the possible solutions. Now, this is a behavior change, and arguably an API break, but it should align better with reasonable expectations of how GSocketClientEvent ought to work. I don't expect it to break anything besides tests that check which order GSocketClientEvent events are emitted in. (Currently, libsoup has such tests, which will need to be updated.) Ideally we would have GLib-level tests as well, but in a concession to pragmatism, it's a lot easier to keep network tests in libsoup. --- gio/gsocketclient.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index 8a663c3de..9df8f290f 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -991,6 +991,7 @@ g_socket_client_connect (GSocketClient *client, { GIOStream *connection = NULL; GSocketAddressEnumerator *enumerator = NULL; + gboolean ever_resolved = FALSE; GError *last_error, *tmp_error; last_error = NULL; @@ -1025,10 +1026,20 @@ g_socket_client_connect (GSocketClient *client, } tmp_error = NULL; - g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVING, - connectable, NULL); + + if (!ever_resolved) + { + g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVING, + connectable, NULL); + } address = g_socket_address_enumerator_next (enumerator, cancellable, &tmp_error); + if (!ever_resolved) + { + g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVED, + connectable, NULL); + ever_resolved = TRUE; + } if (address == NULL) { @@ -1046,8 +1057,6 @@ g_socket_client_connect (GSocketClient *client, _("Unknown error on connect")); break; } - g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVED, - connectable, NULL); using_proxy = (G_IS_PROXY_ADDRESS (address) && client->priv->enable_proxy); @@ -1509,7 +1518,8 @@ enumerator_next_async (GSocketClientAsyncConnectData *data, if (add_task_ref) g_object_ref (data->task); - g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL); + if (!data->enumerated_at_least_once) + g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL); g_debug ("GSocketClient: Starting new address enumeration"); g_socket_address_enumerator_next_async (data->enumerator, data->enumeration_cancellable, @@ -1883,10 +1893,13 @@ g_socket_client_enumerator_callback (GObject *object, return; } - data->enumerated_at_least_once = TRUE; g_debug ("GSocketClient: Address enumeration succeeded"); - g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVED, - data->connectable, NULL); + if (!data->enumerated_at_least_once) + { + g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVED, + data->connectable, NULL); + data->enumerated_at_least_once = TRUE; + } g_clear_error (&data->last_error); From 14f7b5e590f6adc3207019227586d20848274654 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Mon, 5 Oct 2020 12:32:32 -0500 Subject: [PATCH 5/6] gsocketclient: Crash on error if error is missing We should never return unknown errors to the application. This would be a glib bug. I don't think it's currently possible to hit these cases, so asserts should be OK. For this to happen, either (a) a GSocketAddressEnumerator would have to return NULL on its first enumeration, without returning an error, or (b) there would have to be a bug in our GSocketClient logic. Either way, if such a bug were to exist, it would be better to surface it rather than hide it. These changes are actually going to be effectively undone in a subsequent commit, as I'm refactoring the error handling, but the commit history is a bit nicer with two separate commits, so let's go with two. --- gio/gsocketclient.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index 9df8f290f..fb68c0966 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -1053,8 +1053,9 @@ g_socket_client_connect (GSocketClient *client, g_propagate_error (error, last_error); } else - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - _("Unknown error on connect")); + { + g_assert_not_reached (); + } break; } @@ -1879,10 +1880,9 @@ g_socket_client_enumerator_callback (GObject *object, error = data->last_error; data->last_error = NULL; } - else if (!error) + else { - g_set_error_literal (&error, G_IO_ERROR, G_IO_ERROR_FAILED, - _("Unknown error on connect")); + g_assert (error); } complete_connection_with_error (data, error); From b88b3712e0d4474ff55d3b94050285ea08580ddb Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Thu, 8 Oct 2020 18:02:56 -0500 Subject: [PATCH 6/6] gsocketclient: return best errors possible Originally, GSocketClient returned whatever error occured last. Turns out this doesn't work well in practice. Consider the following case: DNS returns an IPv4 and IPv6 address. First we'll connect() to the IPv4 address, and say that succeeds, but TLS is enabled and the TLS handshake fails. Then we try the IPv6 address and receive ENETUNREACH because IPv6 isn't supported. We wind up returning NETWORK_UNREACHABLE even though the address can be pinged and a TLS error would be more appropriate. So instead, we now try to return the error corresponding to the latest attempted GSocketClientEvent in the connection process. TLS errors take precedence over proxy errors, which take precedence over connect() errors, which take precedence over DNS errors. In writing this commit, I made several mistakes that were caught by proxy-test.c, which tests using GSocketClient to make a proxy connection. So although adding a new test to ensure we get the best-possible error would be awkward, at least we have some test coverage for the code that helped avoid introducing bugs. Fixes #2211 --- gio/gsocketclient.c | 211 +++++++++++++++++++++++++++----------------- 1 file changed, 131 insertions(+), 80 deletions(-) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index fb68c0966..ce3c186fb 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -953,6 +953,72 @@ g_socket_client_emit_event (GSocketClient *client, event, connectable, connection); } +/* Originally, GSocketClient returned whatever error occured last. Turns + * out this doesn't work well in practice. Consider the following case: + * DNS returns an IPv4 and IPv6 address. First we'll connect() to the + * IPv4 address, and say that succeeds, but TLS is enabled and the TLS + * handshake fails. Then we try the IPv6 address and receive ENETUNREACH + * because IPv6 isn't supported. We wind up returning NETWORK_UNREACHABLE + * even though the address can be pinged and a TLS error would be more + * appropriate. So instead, we now try to return the error corresponding + * to the latest attempted GSocketClientEvent in the connection process. + * TLS errors take precedence over proxy errors, which take precedence + * over connect() errors, which take precedence over DNS errors. + * + * Note that the example above considers a sync codepath, but this is an + * issue for the async codepath too, where events and errors may occur + * in confusing orders. + */ +typedef struct +{ + GError *tmp_error; + GError *best_error; + GSocketClientEvent best_error_event; +} SocketClientErrorInfo; + +static SocketClientErrorInfo * +socket_client_error_info_new (void) +{ + return g_new0 (SocketClientErrorInfo, 1); +} + +static void +socket_client_error_info_free (SocketClientErrorInfo *info) +{ + g_assert (info->tmp_error == NULL); + g_clear_error (&info->best_error); + g_free (info); +} + +static void +consider_tmp_error (SocketClientErrorInfo *info, + GSocketClientEvent event) +{ + if (info->tmp_error == NULL) + return; + + /* If we ever add more GSocketClientEvents in the future, then we'll + * no longer be able to use >= for this comparison, because future + * events will compare greater than G_SOCKET_CLIENT_COMPLETE. Until + * then, this is convenient. Note G_SOCKET_CLIENT_RESOLVING is 0 so we + * need to use >= here or those errors would never be set. That means + * if we get two errors on the same GSocketClientEvent, we wind up + * preferring the last one, which is fine. + */ + g_assert (event <= G_SOCKET_CLIENT_COMPLETE); + if (event >= info->best_error_event) + { + g_clear_error (&info->best_error); + info->best_error = info->tmp_error; + info->tmp_error = NULL; + info->best_error_event = event; + } + else + { + g_clear_error (&info->tmp_error); + } +} + /** * g_socket_client_connect: * @client: a #GSocketClient. @@ -991,10 +1057,10 @@ g_socket_client_connect (GSocketClient *client, { GIOStream *connection = NULL; GSocketAddressEnumerator *enumerator = NULL; + SocketClientErrorInfo *error_info; gboolean ever_resolved = FALSE; - GError *last_error, *tmp_error; - last_error = NULL; + error_info = socket_client_error_info_new (); if (can_use_proxy (client)) { @@ -1019,21 +1085,19 @@ g_socket_client_connect (GSocketClient *client, if (g_cancellable_is_cancelled (cancellable)) { - g_clear_error (error); - g_clear_error (&last_error); - g_cancellable_set_error_if_cancelled (cancellable, error); + g_clear_error (&error_info->best_error); + g_cancellable_set_error_if_cancelled (cancellable, &error_info->best_error); break; } - tmp_error = NULL; - if (!ever_resolved) { g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVING, connectable, NULL); } address = g_socket_address_enumerator_next (enumerator, cancellable, - &tmp_error); + &error_info->tmp_error); + consider_tmp_error (error_info, G_SOCKET_CLIENT_RESOLVING); if (!ever_resolved) { g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVED, @@ -1043,29 +1107,16 @@ g_socket_client_connect (GSocketClient *client, if (address == NULL) { - if (tmp_error) - { - g_clear_error (&last_error); - g_propagate_error (error, tmp_error); - } - else if (last_error) - { - g_propagate_error (error, last_error); - } - else - { - g_assert_not_reached (); - } + /* Enumeration is finished. */ + g_assert (&error_info->best_error != NULL); break; } using_proxy = (G_IS_PROXY_ADDRESS (address) && client->priv->enable_proxy); - /* clear error from previous attempt */ - g_clear_error (&last_error); - - socket = create_socket (client, address, &last_error); + socket = create_socket (client, address, &error_info->tmp_error); + consider_tmp_error (error_info, G_SOCKET_CLIENT_CONNECTING); if (socket == NULL) { g_object_unref (address); @@ -1077,14 +1128,15 @@ g_socket_client_connect (GSocketClient *client, g_socket_client_emit_event (client, G_SOCKET_CLIENT_CONNECTING, connectable, connection); if (g_socket_connection_connect (G_SOCKET_CONNECTION (connection), - address, cancellable, &last_error)) + address, cancellable, &error_info->tmp_error)) { g_socket_connection_set_cached_remote_address ((GSocketConnection*)connection, NULL); g_socket_client_emit_event (client, G_SOCKET_CLIENT_CONNECTED, connectable, connection); } else { - clarify_connect_error (last_error, connectable, address); + clarify_connect_error (error_info->tmp_error, connectable, address); + consider_tmp_error (error_info, G_SOCKET_CLIENT_CONNECTING); g_object_unref (connection); connection = NULL; } @@ -1105,9 +1157,10 @@ g_socket_client_connect (GSocketClient *client, g_critical ("Trying to proxy over non-TCP connection, this is " "most likely a bug in GLib IO library."); - g_set_error_literal (&last_error, + g_set_error_literal (&error_info->tmp_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, _("Proxying over a non-TCP connection is not supported.")); + consider_tmp_error (error_info, G_SOCKET_CLIENT_PROXY_NEGOTIATING); g_object_unref (connection); connection = NULL; @@ -1125,7 +1178,9 @@ g_socket_client_connect (GSocketClient *client, connection, proxy_addr, cancellable, - &last_error); + &error_info->tmp_error); + consider_tmp_error (error_info, G_SOCKET_CLIENT_PROXY_NEGOTIATING); + g_object_unref (connection); connection = proxy_connection; g_object_unref (proxy); @@ -1135,9 +1190,10 @@ g_socket_client_connect (GSocketClient *client, } else { - g_set_error (&last_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, + g_set_error (&error_info->tmp_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, _("Proxy protocol “%s” is not supported."), protocol); + consider_tmp_error (error_info, G_SOCKET_CLIENT_PROXY_NEGOTIATING); g_object_unref (connection); connection = NULL; } @@ -1147,7 +1203,7 @@ g_socket_client_connect (GSocketClient *client, { GIOStream *tlsconn; - tlsconn = g_tls_client_connection_new (connection, connectable, &last_error); + tlsconn = g_tls_client_connection_new (connection, connectable, &error_info->tmp_error); g_object_unref (connection); connection = tlsconn; @@ -1157,16 +1213,21 @@ g_socket_client_connect (GSocketClient *client, client->priv->tls_validation_flags); g_socket_client_emit_event (client, G_SOCKET_CLIENT_TLS_HANDSHAKING, connectable, connection); if (g_tls_connection_handshake (G_TLS_CONNECTION (tlsconn), - cancellable, &last_error)) + cancellable, &error_info->tmp_error)) { g_socket_client_emit_event (client, G_SOCKET_CLIENT_TLS_HANDSHAKED, connectable, connection); } else { + consider_tmp_error (error_info, G_SOCKET_CLIENT_TLS_HANDSHAKING); g_object_unref (tlsconn); connection = NULL; } } + else + { + consider_tmp_error (error_info, G_SOCKET_CLIENT_TLS_HANDSHAKING); + } } if (connection && !G_IS_SOCKET_CONNECTION (connection)) @@ -1183,6 +1244,10 @@ g_socket_client_connect (GSocketClient *client, } g_object_unref (enumerator); + if (!connection) + g_propagate_error (error, g_steal_pointer (&error_info->best_error)); + socket_client_error_info_free (error_info); + g_socket_client_emit_event (client, G_SOCKET_CLIENT_COMPLETE, connectable, connection); return G_SOCKET_CONNECTION (connection); } @@ -1360,7 +1425,7 @@ typedef struct GSList *connection_attempts; GSList *successful_connections; - GError *last_error; + SocketClientErrorInfo *error_info; gboolean enumerated_at_least_once; gboolean enumeration_completed; @@ -1380,7 +1445,7 @@ g_socket_client_async_connect_data_free (GSocketClientAsyncConnectData *data) g_slist_free_full (data->connection_attempts, connection_attempt_unref); g_slist_free_full (data->successful_connections, connection_attempt_unref); - g_clear_error (&data->last_error); + g_clear_pointer (&data->error_info, socket_client_error_info_free); g_slice_free (GSocketClientAsyncConnectData, data); } @@ -1502,14 +1567,6 @@ g_socket_client_enumerator_callback (GObject *object, GAsyncResult *result, gpointer user_data); -static void -set_last_error (GSocketClientAsyncConnectData *data, - GError *error) -{ - g_clear_error (&data->last_error); - data->last_error = error; -} - static void enumerator_next_async (GSocketClientAsyncConnectData *data, gboolean add_task_ref) @@ -1540,7 +1597,7 @@ g_socket_client_tls_handshake_callback (GObject *object, if (g_tls_connection_handshake_finish (G_TLS_CONNECTION (object), result, - &data->last_error)) + &data->error_info->tmp_error)) { g_object_unref (attempt->connection); attempt->connection = G_IO_STREAM (object); @@ -1553,7 +1610,9 @@ g_socket_client_tls_handshake_callback (GObject *object, { g_object_unref (object); connection_attempt_unref (attempt); - g_debug ("GSocketClient: TLS handshake failed: %s", data->last_error->message); + + g_debug ("GSocketClient: TLS handshake failed: %s", data->error_info->tmp_error->message); + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_TLS_HANDSHAKING); try_next_connection_or_finish (data, TRUE); } } @@ -1573,7 +1632,7 @@ g_socket_client_tls_handshake (ConnectionAttempt *attempt) g_debug ("GSocketClient: Starting TLS handshake"); tlsconn = g_tls_client_connection_new (attempt->connection, data->connectable, - &data->last_error); + &data->error_info->tmp_error); if (tlsconn) { g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (tlsconn), @@ -1588,6 +1647,8 @@ g_socket_client_tls_handshake (ConnectionAttempt *attempt) else { connection_attempt_unref (attempt); + + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_TLS_HANDSHAKING); try_next_connection_or_finish (data, TRUE); } } @@ -1603,19 +1664,19 @@ g_socket_client_proxy_connect_callback (GObject *object, g_object_unref (attempt->connection); attempt->connection = g_proxy_connect_finish (G_PROXY (object), result, - &data->last_error); + &data->error_info->tmp_error); if (attempt->connection) { g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_PROXY_NEGOTIATED, data->connectable, attempt->connection); + g_socket_client_tls_handshake (attempt); } else { connection_attempt_unref (attempt); - try_next_connection_or_finish (data, TRUE); - return; - } - g_socket_client_tls_handshake (attempt); + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_PROXY_NEGOTIATING); + try_next_connection_or_finish (data, TRUE); + } } static void @@ -1683,9 +1744,10 @@ try_next_successful_connection (GSocketClientAsyncConnectData *data) g_critical ("Trying to proxy over non-TCP connection, this is " "most likely a bug in GLib IO library."); - g_set_error_literal (&data->last_error, + g_set_error_literal (&data->error_info->tmp_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, _("Proxying over a non-TCP connection is not supported.")); + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_PROXY_NEGOTIATING); } else if (g_hash_table_contains (data->client->priv->app_proxies, protocol)) { @@ -1712,11 +1774,10 @@ try_next_successful_connection (GSocketClientAsyncConnectData *data) } else { - g_clear_error (&data->last_error); - - g_set_error (&data->last_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, + g_set_error (&data->error_info->tmp_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, _("Proxy protocol “%s” is not supported."), protocol); + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_PROXY_NEGOTIATING); } data->connection_in_progress = FALSE; @@ -1747,7 +1808,7 @@ try_next_connection_or_finish (GSocketClientAsyncConnectData *data, return; } - complete_connection_with_error (data, data->last_error); + complete_connection_with_error (data, g_steal_pointer (&data->error_info->best_error)); } static void @@ -1757,7 +1818,6 @@ g_socket_client_connected_callback (GObject *source, { ConnectionAttempt *attempt = user_data; GSocketClientAsyncConnectData *data = attempt->data; - GError *error = NULL; if (task_completed_or_cancelled (data) || g_cancellable_is_cancelled (attempt->cancellable)) { @@ -1773,20 +1833,20 @@ g_socket_client_connected_callback (GObject *source, } if (!g_socket_connection_connect_finish (G_SOCKET_CONNECTION (source), - result, &error)) + result, &data->error_info->tmp_error)) { if (!g_cancellable_is_cancelled (attempt->cancellable)) { - clarify_connect_error (error, data->connectable, attempt->address); - set_last_error (data, error); - g_debug ("GSocketClient: Connection attempt failed: %s", error->message); + clarify_connect_error (data->error_info->tmp_error, data->connectable, attempt->address); + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_CONNECTING); + g_debug ("GSocketClient: Connection attempt failed: %s", data->error_info->tmp_error->message); connection_attempt_remove (attempt); connection_attempt_unref (attempt); try_next_connection_or_finish (data, FALSE); } else /* Silently ignore cancelled attempts */ { - g_clear_error (&error); + g_clear_error (&data->error_info->tmp_error); g_object_unref (data->task); connection_attempt_unref (attempt); } @@ -1844,7 +1904,6 @@ g_socket_client_enumerator_callback (GObject *object, GSocketAddress *address = NULL; GSocket *socket; ConnectionAttempt *attempt; - GError *error = NULL; if (task_completed_or_cancelled (data)) { @@ -1853,7 +1912,7 @@ g_socket_client_enumerator_callback (GObject *object, } address = g_socket_address_enumerator_next_finish (data->enumerator, - result, &error); + result, &data->error_info->tmp_error); if (address == NULL) { if (G_UNLIKELY (data->enumeration_completed)) @@ -1862,7 +1921,7 @@ g_socket_client_enumerator_callback (GObject *object, data->enumeration_completed = TRUE; g_debug ("GSocketClient: Address enumeration completed (out of addresses)"); - /* As per API docs: We only care about error if its the first call, + /* As per API docs: We only care about error if it's the first call, after that the enumerator is done. Note that we don't care about cancellation errors because @@ -1873,19 +1932,11 @@ g_socket_client_enumerator_callback (GObject *object, if ((data->enumerated_at_least_once && !data->connection_attempts && !data->connection_in_progress) || !data->enumerated_at_least_once) { - g_debug ("GSocketClient: Address enumeration failed: %s", error ? error->message : NULL); - if (data->last_error) - { - g_clear_error (&error); - error = data->last_error; - data->last_error = NULL; - } - else - { - g_assert (error); - } - - complete_connection_with_error (data, error); + g_debug ("GSocketClient: Address enumeration failed: %s", + data->error_info->tmp_error ? data->error_info->tmp_error->message : NULL); + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_RESOLVING); + g_assert (data->error_info->best_error); + complete_connection_with_error (data, g_steal_pointer (&data->error_info->best_error)); } /* Enumeration should never trigger again, drop our ref */ @@ -1901,12 +1952,11 @@ g_socket_client_enumerator_callback (GObject *object, data->enumerated_at_least_once = TRUE; } - g_clear_error (&data->last_error); - - socket = create_socket (data->client, address, &data->last_error); + socket = create_socket (data->client, address, &data->error_info->tmp_error); if (socket == NULL) { g_object_unref (address); + consider_tmp_error (data->error_info, G_SOCKET_CLIENT_CONNECTING); enumerator_next_async (data, FALSE); return; } @@ -1978,6 +2028,7 @@ g_socket_client_connect_async (GSocketClient *client, data = g_slice_new0 (GSocketClientAsyncConnectData); data->client = client; data->connectable = g_object_ref (connectable); + data->error_info = socket_client_error_info_new (); if (can_use_proxy (client)) {