From 004570a377b2df355064d48afc54375690c3cdb0 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 16 Jun 2020 15:14:11 -0400 Subject: [PATCH] Add HTTP responses to FileTransferErrors --- src/libstore/filetransfer.cc | 26 ++++++++++++++++++++------ src/libstore/filetransfer.hh | 5 +++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 9566e0ae9..d89f8388f 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -122,7 +122,7 @@ struct curlFileTransfer : public FileTransfer if (requestHeaders) curl_slist_free_all(requestHeaders); try { if (!done) - fail(FileTransferError(Interrupted, "download of '%s' was interrupted", request.uri)); + fail(FileTransferError(Interrupted, nullptr, "download of '%s' was interrupted", request.uri)); } catch (...) { ignoreException(); } @@ -152,8 +152,18 @@ struct curlFileTransfer : public FileTransfer size_t realSize = size * nmemb; result.bodySize += realSize; - if (!decompressionSink) + if (!decompressionSink) { decompressionSink = makeDecompressionSink(encoding, finalSink); + if (! successfulStatuses.count(getHTTPStatus())) { + // In this case we want to construct a TeeSink, to keep + // the response around (which we figure won't be big + // like an actual download should be) to improve error + // messages. + decompressionSink = std::make_shared>>( + ref{ decompressionSink } + ); + } + } (*decompressionSink)((unsigned char *) contents, realSize); @@ -408,16 +418,20 @@ struct curlFileTransfer : public FileTransfer attempt++; + std::shared_ptr response; + if (decompressionSink) + if (auto teeSink = std::dynamic_pointer_cast>>(decompressionSink)) + response = teeSink->data; auto exc = code == CURLE_ABORTED_BY_CALLBACK && _isInterrupted - ? FileTransferError(Interrupted, fmt("%s of '%s' was interrupted", request.verb(), request.uri)) + ? FileTransferError(Interrupted, response, fmt("%s of '%s' was interrupted", request.verb(), request.uri)) : httpStatus != 0 - ? FileTransferError(err, + ? FileTransferError(err, response, fmt("unable to %s '%s': HTTP error %d", request.verb(), request.uri, httpStatus) + (code == CURLE_OK ? "" : fmt(" (curl error: %s)", curl_easy_strerror(code))) ) - : FileTransferError(err, + : FileTransferError(err, response, fmt("unable to %s '%s': %s (%d)", request.verb(), request.uri, curl_easy_strerror(code), code)); @@ -675,7 +689,7 @@ struct curlFileTransfer : public FileTransfer auto s3Res = s3Helper.getObject(bucketName, key); FileTransferResult res; if (!s3Res.data) - throw FileTransferError(NotFound, fmt("S3 object '%s' does not exist", request.uri)); + throw FileTransferError(NotFound, nullptr, fmt("S3 object '%s' does not exist", request.uri)); res.data = s3Res.data; callback(std::move(res)); #else diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index 11dca2fe0..8e31a9e42 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -103,9 +103,10 @@ class FileTransferError : public Error { public: FileTransfer::Error error; + std::shared_ptr response; // intentionally optional template - FileTransferError(FileTransfer::Error error, const Args & ... args) - : Error(args...), error(error) + FileTransferError(FileTransfer::Error error, std::shared_ptr response, const Args & ... args) + : Error(args...), error(error), response(response) { } };