From cd794650199b5a208ad602919fd9b6e6a16f1bfc Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Sun, 15 Oct 2023 17:02:03 +0200 Subject: [PATCH] HTTPFS: Move from HTTPException to base class IOException This commit might need to be backported (at least for MacOS builds) to previous released httpfs extension binaries (most affected are v0.9.1 due to how this is put onto the spotlight due to #9286) to fix issue like #9340 My idea on how to proceed is as follow: 1. this can be merged into duckdb and have nightly binaries (and connected extensions) throwing IOExceptions (that are properly handled) instead of specialized payload of HTTPExtensions This allows to test (on CI and on nightlies) whether this solves the actual problem This also allows, independently, to explore how to build specific extensions with a custom patch (this commit is basically the patch) 2. Fixing the handling of HTTPExtension in core duckdb. Potentially this could mean axing the HTTP-specific payload and moving to a generic textual plus custom payload extracted via virtual funtion calls --- extension/httpfs/httpfs.cpp | 8 ++++---- extension/httpfs/s3fs.cpp | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/extension/httpfs/httpfs.cpp b/extension/httpfs/httpfs.cpp index 069b62e0dc44..f8e114e0ef6e 100644 --- a/extension/httpfs/httpfs.cpp +++ b/extension/httpfs/httpfs.cpp @@ -122,7 +122,7 @@ RunRequestWithRetry(const std::function &r if (caught_e) { std::rethrow_exception(caught_e); } else if (err == duckdb_httplib_openssl::Error::Success) { - throw HTTPException(response, "Request returned HTTP %d for HTTP %s to '%s'", status, method, url); + throw IOException("Request returned HTTP %d for HTTP %s to '%s'", status, method, url); } else { throw IOException("%s error for HTTP %s to '%s'", to_string(err), method, url); } @@ -310,7 +310,7 @@ unique_ptr HTTPFileSystem::GetRangeRequest(FileHandle &handle, error += " This could mean the file was changed. Try disabling the duckdb http metadata cache " "if enabled, and confirm the server supports range requests."; } - throw HTTPException(response, error); + throw IOException(error); } if (response.status < 300) { // done redirecting out_offset = 0; @@ -600,7 +600,7 @@ void HTTPFileHandle::Initialize(FileOpener *opener) { } res = std::move(range_res); } else { - throw HTTPException(*res, "Unable to connect to URL \"%s\": %s (%s)", res->http_url, + throw IOException("Unable to connect to URL \"%s\": %s (%s)", res->http_url, to_string(res->code), res->error); } } @@ -634,7 +634,7 @@ void HTTPFileHandle::Initialize(FileOpener *opener) { // Try to fully download the file first auto full_download_result = hfs.GetRequest(*this, path, {}); if (full_download_result->code != 200) { - throw HTTPException(*res, "Full download failed to to URL \"%s\": %s (%s)", + throw IOException("Full download failed to to URL \"%s\": %s (%s)", full_download_result->http_url, to_string(full_download_result->code), full_download_result->error); } diff --git a/extension/httpfs/s3fs.cpp b/extension/httpfs/s3fs.cpp index 563e016a2589..ef7a93136e09 100644 --- a/extension/httpfs/s3fs.cpp +++ b/extension/httpfs/s3fs.cpp @@ -303,7 +303,7 @@ void S3FileSystem::UploadBuffer(S3FileHandle &file_handle, shared_ptrcode != 200) { - throw HTTPException(*res, "Unable to connect to URL %s %s (HTTP code %s)", res->http_url, res->error, + throw IOException("Unable to connect to URL %s %s (HTTP code %s)", res->http_url, res->error, to_string(res->code)); } @@ -438,7 +438,7 @@ void S3FileSystem::FinalizeMultipartUpload(S3FileHandle &file_handle) { auto open_tag_pos = result.find("code); + throw IOException("Unexpected response during S3 multipart upload finalization: %d", res->code); } file_handle.upload_finalized = true; } @@ -1036,7 +1036,7 @@ string AWSListObjectV2::Request(string &path, HTTPParams &http_params, S3AuthPar listobjectv2_url.c_str(), *headers, [&](const duckdb_httplib_openssl::Response &response) { if (response.status >= 400) { - throw HTTPException(response, "HTTP GET error on '%s' (HTTP %d)", listobjectv2_url, response.status); + throw IOException("HTTP GET error on '%s' (HTTP %d)", listobjectv2_url, response.status); } return true; },