Skip to content

Use HTTPlug instead of Guzzle#6

Merged
jasonbosco merged 5 commits intotypesense:masterfrom
babeuloula:use-httplug
Dec 22, 2020
Merged

Use HTTPlug instead of Guzzle#6
jasonbosco merged 5 commits intotypesense:masterfrom
babeuloula:use-httplug

Conversation

@babeuloula
Copy link
Copy Markdown
Contributor

@babeuloula babeuloula commented Dec 5, 2020

Change Summary

I want to use your library but I use symfony/http-client. And I can't use it because you use Guzzle. So I made this PR in order to use HTTPlug instead.

I fixed a phpcs error and Late Static Bindings.

PR Checklist

@babeuloula
Copy link
Copy Markdown
Contributor Author

Hi,

Thanks for the approval.
Do you know when you're going to merge this PR?

@jasonbosco jasonbosco self-requested a review December 12, 2020 03:36
Copy link
Copy Markdown
Member

@jasonbosco jasonbosco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@babeuloula Thank you for this PR!

I did some final sanity checks by running the examples and noticed this error:

$ php collection_operations.php 
<pre>--------Create Collection-------
Header values must be RFC 7230 compatible strings
$

My PHP knowledge is a little outdated now... Any idea what's wrong? Do the examples run ok in your environment?

@babeuloula
Copy link
Copy Markdown
Contributor Author

@babeuloula Thank you for this PR!

I did some final sanity checks by running the examples and noticed this error:

$ php collection_operations.php 
<pre>--------Create Collection-------
Header values must be RFC 7230 compatible strings
$

My PHP knowledge is a little outdated now... Any idea what's wrong? Do the examples run ok in your environment?

Yes, you need to json_encode your body. I've fix that.

Copy link
Copy Markdown
Member

@jasonbosco jasonbosco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@babeuloula That error has now gone away, but it now looks like the API key is not being passed to the server. Here's the error I get:

$ php alias_operations.php 
<pre>--------Create Collection-------
Forbidden - a valid `x-typesense-api-key` header must be sent.

I've verified that the server and client are configured with the same API key

@babeuloula
Copy link
Copy Markdown
Contributor Author

@babeuloula That error has now gone away, but it now looks like the API key is not being passed to the server. Here's the error I get:

$ php alias_operations.php 
<pre>--------Create Collection-------
Forbidden - a valid `x-typesense-api-key` header must be sent.

I've verified that the server and client are configured with the same API key

You have more logs ? Is it possible to have a debug mode on typesense-server ? I don't know why you have this message. I did not changed the logic about the API key.

@jasonbosco
Copy link
Copy Markdown
Member

@babeuloula I dug in a little big more with a debugger and I see the issue:

I set a breakpoint on this line and stepped into the send method:

$response = $this->client->send(\strtoupper($method), $url, $reqOp);

which then takes me to this piece of code in the HttpMethodsClient class:

https://github.com/php-http/utils/blob/cf5d9efefce93994389f68f714d84b4f065a7a20/src/HttpMethodsClient.php#L186-L194

    public function send($method, $uri, array $headers = [], $body = null)
    {
        return $this->sendRequest($this->messageFactory->createRequest(
            $method,
            $uri,
            $headers,
            $body
        ));
    }

Notice how the signature of this method is different from how it's being called in ApiCall.

So the headers parameter that this send method receives ends up looking like this:

Screen Shot 2020-12-17 at 6 40 02 PM

If you trace this further down the stack, this nested structure eventually causes the X-TYPESENSE-API-KEY from not being passed through to the server.

@babeuloula
Copy link
Copy Markdown
Contributor Author

@jasonbosco Sorry, now it's fixed.

For monolog, I've change from "^1.0" to "^1.0|^2.1" to allow users to use monolog 2.

Copy link
Copy Markdown
Member

@jasonbosco jasonbosco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@babeuloula The examples now work well! Thank you for making those changes.

I just noticed two additional things, could you take a look?

Comment thread src/ApiCall.php Outdated
Comment thread src/Lib/Configuration.php
@jasonbosco jasonbosco merged commit 88dada1 into typesense:master Dec 22, 2020
@jasonbosco
Copy link
Copy Markdown
Member

Thank you again for the PR @babeuloula!

@jasonbosco
Copy link
Copy Markdown
Member

Published this as v4.3.0

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

Successfully merging this pull request may close these issues.

3 participants