From 476493dbf5d5f66db3fcfe305430972c5accf10f Mon Sep 17 00:00:00 2001 From: Dan Peebles Date: Mon, 25 Sep 2017 12:36:01 -0700 Subject: Reverse retry logic to retry in all but a few cases It was getting too much like whac-a-mole listing all the retriable error conditions, so we now retry by default and list the cases where retrying is almost certainly hopeless. --- src/libstore/download.cc | 57 +++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 20 deletions(-) (limited to 'src/libstore/download.cc') diff --git a/src/libstore/download.cc b/src/libstore/download.cc index 3f5e744dde19..608b8fd399b4 100644 --- a/src/libstore/download.cc +++ b/src/libstore/download.cc @@ -278,26 +278,43 @@ struct CurlDownloader : public Downloader callFailure(failure, std::current_exception()); } } else { - Error err = - (httpStatus == 404 || code == CURLE_FILE_COULDNT_READ_FILE) ? NotFound : - httpStatus == 403 ? Forbidden : - (httpStatus == 408 || httpStatus == 500 || httpStatus == 503 - || httpStatus == 504 || httpStatus == 522 || httpStatus == 524 - || code == CURLE_COULDNT_RESOLVE_HOST - || code == CURLE_RECV_ERROR - - // this seems to occur occasionally for retriable reasons, and shows up in an error like this: - // curl: (23) Failed writing body (315 != 16366) - || code == CURLE_WRITE_ERROR - - // this is a generic SSL failure that in some cases (e.g., certificate error) is permanent but also appears in transient cases, so we consider it retryable - || code == CURLE_SSL_CONNECT_ERROR -#if LIBCURL_VERSION_NUM >= 0x073200 - || code == CURLE_HTTP2 - || code == CURLE_HTTP2_STREAM -#endif - ) ? Transient : - Misc; + // We treat most errors as transient, but won't retry when hopeless + Error err = Transient; + + if (httpStatus == 404 || code == CURLE_FILE_COULDNT_READ_FILE) { + // The file is definitely not there + err = NotFound; + } else if (httpStatus == 401 || httpStatus == 403 || httpStatus == 407) { + // Don't retry on authentication/authorization failures + err = Forbidden; + } else if (httpStatus >= 400 && httpStatus < 500 && httpStatus != 408) { + // Most 4xx errors are client errors and are probably not worth retrying: + // * 408 means the server timed out waiting for us, so we try again + err = Misc; + } else if (httpStatus == 501 || httpStatus == 505 || httpStatus == 511) { + // Let's treat most 5xx (server) errors as transient, except for a handful: + // * 501 not implemented + // * 505 http version not supported + // * 511 we're behind a captive portal + err = Misc; + } else { + // Don't bother retrying on certain cURL errors either + switch (code) { + case CURLE_FAILED_INIT: + case CURLE_NOT_BUILT_IN: + case CURLE_REMOTE_ACCESS_DENIED: + case CURLE_FILE_COULDNT_READ_FILE: + case CURLE_FUNCTION_NOT_FOUND: + case CURLE_ABORTED_BY_CALLBACK: + case CURLE_BAD_FUNCTION_ARGUMENT: + case CURLE_INTERFACE_FAILED: + case CURLE_UNKNOWN_OPTION: + err = Misc; + break; + default: // Shut up warnings + break; + } + } attempt++; -- cgit 1.4.1