Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CURL client: GetResponseCode always returns -1 (REQUEST_NOT_MADE) in HeadersReceivedEventHandler #3234

Open
1 task
bytechunk opened this issue Jan 1, 2025 · 4 comments
Labels
bug This issue is a bug. p2 This is a standard priority issue

Comments

@bytechunk
Copy link

Describe the bug

When using HeadersReceivedEventHandler (request.SetHeadersReceivedEventHandler), the response object passed on handler invocation is not fully set according to HTTP header section content.

In my case, response->GetResponseCode ( ) always returns -1 (REQUEST_NOT_MADE) whatever the HTTP status code returned by the server in its header section.

I did not check values returned by other methods of the response object (I only had a kick look at GetContentType) at that time but it seems they may return the expected values because they rely on HTTP header keys/values appended while getting header data.

Regression Issue

  • Select this option if this issue appears to be a regression.

Expected Behavior

The response code should be set according to HTTP status code returned by the server before calling the HeadersReceivedEventHandler.

Current Behavior

calling response->GetResponseCode () in HeadersReceivedEventHandler always returns -1

Reproduction Steps

bool AwsDoc::S3::getObject(const Aws::String &objectKey,
                           const Aws::String &fromBucket,
                           const Aws::S3::S3ClientConfiguration &clientConfig) {
    Aws::S3::S3Client client(clientConfig);

    Aws::S3::Model::GetObjectRequest request;
    request.SetBucket(fromBucket);
    request.SetKey("one_object_that_does_not_exist_in_the_bucket"); // <== we should get 404 when calling GetResponseCode

    request.SetHeadersReceivedEventHandler (
      [] (
        const Aws::Http::HttpRequest * request,
        Aws::Http::HttpResponse * response
      ) {
        ::std::cout <<  "response headers received: "
            << response->GetResponseCode ( )
           << ::std::endl; // <== current implementation is always "response headers received: -1"
      }
    );

    Aws::S3::Model::GetObjectOutcome outcome =
            client.GetObject(request);

    if (!outcome.IsSuccess()) {
        const Aws::S3::S3Error &err = outcome.GetError();
        std::cerr << "Error: getObject: " <<
                  err.GetExceptionName() << ": " << err.GetMessage() << std::endl;
    } else {
        std::cout << "Successfully retrieved '" << objectKey << "' from '"
                  << fromBucket << "'." << std::endl;
    }

    return outcome.IsSuccess();
}

Possible Solution

call SetResponseCode before calling HeadersReceivedEventHandler

Additional Information/Context

Client: CURL
OS: linux / debian
aws-sdk-cpp: main branch

I assume this bug is applicable to all S3 actions (not only GetObject) since the bug is in the low level HTTP client implementation (CURL based).

I have a fix to propose if it can help.

AWS CPP SDK version used

1.11.476

Compiler and Version used

c++ (Debian 12.2.0-14) 12.2.0

Operating System and version

debian 12.8

@bytechunk bytechunk added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 1, 2025
@sbera87
Copy link
Contributor

sbera87 commented Jan 8, 2025

Hello, I confirm your observation and in my opinion the real reason the response code is not available before invoking of the HeaderReceivedEventHandler is the lack of true async support in the codebase using curl http client.

Per current implementation, the write callback function is registered here:

curl_easy_setopt(connectionHandle, CURLOPT_HEADERFUNCTION, WriteHeader);

Then, we call this blocking API which will internally invoke the write callbacks as data is received from the server but the response code is only available till after the completion of the API.

CURLcode curlResponseCode = curl_easy_perform(connectionHandle);

@bytechunk
Copy link
Author

bytechunk commented Jan 8, 2025

Effectively, my idea is to handle it in the WriteHeader curl callback instead of waiting for curl_easy_perform to return.

Could you have a look at https://github.com/bytechunk/aws-sdk-cpp/compare/fix_GetResponseCode_in_HeadersReceivedEventHandler

By doing it this way, it works for sync and async calls.

@sbera87
Copy link
Contributor

sbera87 commented Jan 8, 2025

I am not opposed to it and shall review it closer. Could you please let us know what is the use case for which you may need the response code prior to the header received callback invocation? It will help use to review it with that perspective better

@jmklix jmklix added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jan 8, 2025
@bytechunk
Copy link
Author

bytechunk commented Jan 10, 2025

In my project, object contents are not available as files and I cannot write object contents on disk when getting object contents.

Everything is handled on the fly using custom STL stream classes.

When trying to put new objects, there is no issue. My problem occurs when I try to get objects from the bucket.

To process object contents on the fly, I register my own stream factory using ::Aws::S3::Model::GetObjectRequest::SetResponseStreamFactory.
Everything works fine in nominal cases (when the request returns with no error), my stream object gets allocated and the object content is processed as expected.

However, when GetObjectRequest returns with error, then my stream object starts processing the response body that does not contain the expected content but a content describing the error (XML).

To solve this, I tried to use ::Aws::S3::Model::GetObjectRequest::SetHeadersReceivedEventHandler so that I could redirect the body content (before receiving the body) depending on the response status code.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

3 participants