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

Guzzle plaintext response error #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dumityty
Copy link

This is a PR to fix the issue raised in #152 which I've also encountered after applying the patch from #139 to be able to use the WorkersKV endpoints.

The fix is quite basic and there might be a better of doing this, but from what I can see whenever there's either success or error responses they are always in JSON format, except for the one random endpoint to 'Read key-value pair ' (https://api.cloudflare.com/#workers-kv-namespace-read-key-value-pair) which returns the value of the key in plain text.
So to get around that I just checked if the first character of the response is a { or not, and if it's not then skip all the json code below otherwise it fails with a 'json syntax' error since it's a plain text.

I ran all the tests and they passed, however I didn't write a new one as the change is to the Guzzle adapter which already has all the tests.
I guess I could try writing one to cover the 'plain text' response from the API.

@deuill
Copy link
Contributor

deuill commented Mar 8, 2021

I'll need to investigate this, as json_decode on the responses from that sample endpoint (e.g. "Some Value", note the double-quotes) are correctly decoded from a bare JSON string into a PHP string. That is to say, the responses for that endpoint are indeed valid JSON, albeit not wrapped in an object as is typically the case.

@deuill
Copy link
Contributor

deuill commented Mar 10, 2021

I've opened a perhaps more concrete fix to the issue you're seeing here under #164.

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.

2 participants