Skip to content

Commit

Permalink
HTTPFS: Move from HTTPException to base class IOException
Browse files Browse the repository at this point in the history
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 duckdb#9286) to fix issue like duckdb#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
  • Loading branch information
carlopi committed Oct 15, 2023
1 parent 6eeb682 commit cd79465
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
8 changes: 4 additions & 4 deletions extension/httpfs/httpfs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ RunRequestWithRetry(const std::function<duckdb_httplib_openssl::Result(void)> &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);
}
Expand Down Expand Up @@ -310,7 +310,7 @@ unique_ptr<ResponseWrapper> 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;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions extension/httpfs/s3fs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ void S3FileSystem::UploadBuffer(S3FileHandle &file_handle, shared_ptr<S3WriteBuf
query_param);

if (res->code != 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));
}

Expand Down Expand Up @@ -438,7 +438,7 @@ void S3FileSystem::FinalizeMultipartUpload(S3FileHandle &file_handle) {

auto open_tag_pos = result.find("<CompleteMultipartUploadResult", 0);
if (open_tag_pos == string::npos) {
throw HTTPException(*res, "Unexpected response during S3 multipart upload finalization: %d", res->code);
throw IOException("Unexpected response during S3 multipart upload finalization: %d", res->code);
}
file_handle.upload_finalized = true;
}
Expand Down Expand Up @@ -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;
},
Expand Down

0 comments on commit cd79465

Please sign in to comment.