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

Missing Parameter in returned response from the AWS Handler #73

Closed
HalimNaim opened this issue Jan 9, 2020 · 5 comments
Closed

Missing Parameter in returned response from the AWS Handler #73

HalimNaim opened this issue Jan 9, 2020 · 5 comments

Comments

@HalimNaim
Copy link

HalimNaim commented Jan 9, 2020

I've Version 4.1.1 of this package with version 7.5 for elasticsearch.

On this line in the Factory a response is returned from the AWS handler. It is missing a property named primary_port in transfer_stats that is required by the elasticsearch library here

Factory.php

 // Convert the PSR-7 response to a RingPHP response
 return new \GuzzleHttp\Ring\Future\CompletedFutureArray([
        'status' => $response->getStatusCode(),
        'headers' => $response->getHeaders(),
        'body' => $response->getBody()->detach(),
        'transfer_stats' => ['total_time' => 0],
        'effective_url' => (string)$psr7Request->getUri(),
]);

Connection.php

 public function logRequestSuccess(array $request, array $response): void
    {
        $this->log->debug('Request Body', array($request['body']));
        $this->log->info(
            'Request Success:',
            array(
                'method'    => $request['http_method'],
                'uri'       => $response['effective_url'],
                'port'      => $response['transfer_stats']['primary_port'],
                'headers'   => $request['headers'],
                'HTTP code' => $response['status'],
                'duration'  => $response['transfer_stats']['total_time'],
            )
        );
        $this->log->debug('Response', array($response['body']));

Added in this commit
On 2019-09-19

@cviebrock
Copy link
Contributor

I didn't write the AWS portion of this package, so I'm a bit unclear as to what's needed. Should I be changing the Factory to do something like:

                    return new \GuzzleHttp\Ring\Future\CompletedFutureArray([
                        'status'         => $response->getStatusCode(),
                        'headers'        => $response->getHeaders(),
                        'body'           => $response->getBody()
                                                     ->detach(),
                        'transfer_stats' => [
                            'total_time' => 0,
                            'primary_port' => '',
                        ],
                        'effective_url'  => (string)$psr7Request->getUri(),
                    ]);

I'm not sure what that value should be. Could you make a PR to fix this?

@HalimNaim HalimNaim changed the title Missing Parameter en returned response from the AWS Handler Missing Parameter in returned response from the AWS Handler Jan 20, 2020
@HalimNaim
Copy link
Author

Yeah sure, I would love to.

@HalimNaim
Copy link
Author

HalimNaim commented Jan 20, 2020

Hey, they have fixed it on their side 6 days ago. Here

The port is the port used to connect to ES. For the AWS case I think it can be inferred from the scheme used. http or https.

I can still write the PR if you would like to add it.

@cviebrock
Copy link
Contributor

If you think it's necessary now, then a PR would be great. Thanks!

@cviebrock
Copy link
Contributor

Closing due to no response. Feel free to re-open or comment if you are still having issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants