Skip to content
This repository was archived by the owner on Jan 13, 2022. It is now read-only.

The sdk not working at all - can not parse http/2 response code #1091

Closed
hanakivan opened this issue Dec 4, 2018 · 12 comments
Closed

The sdk not working at all - can not parse http/2 response code #1091

hanakivan opened this issue Dec 4, 2018 · 12 comments

Comments

@hanakivan
Copy link

hanakivan commented Dec 4, 2018

The faulty method is this:

public function setHttpResponseCodeFromHeader($rawResponseHeader) { preg_match('|HTTP/\d\.\d\s+(\d+)\s+.*|', $rawResponseHeader, $match); $this->httpResponseCode = (int)$match[1]; }

it will parse only HTTP/1.1, will not parse HTTP/2 - this makes current version of sdk useless

The faulty version is 5.6.3 that is available on packagist.

@ilkin007
Copy link

ilkin007 commented Dec 4, 2018

Having the same issue...

@ilkin007
Copy link

ilkin007 commented Dec 4, 2018

In case we change the code to:

public function setHttpResponseCodeFromHeader($rawResponseHeader)
{
    preg_match('|HTTP/\d\s+(\d+)|', $rawResponseHeader, $match);
    $this->httpResponseCode = (int)$match[1];
}

It works again :) Just changed regular expression to accept HTTP/2 .
But it would be cool if we add it for both versions of HTTP (1.1 and 2)

@fbourigault
Copy link

I have the same issue. I moved to the stream http_client_handler so it doesn't use h2 to perform requests.

@jakubkulhan
Copy link

Ouch, just experienced this bug in our app. Any time frame when it will be fixed?

@finwe
Copy link

finwe commented Dec 11, 2018

PR fixing this was merged almost a month ago. #1079

@julienbourdeau
Copy link

I'm hoping we can get a patch version rather than wait for 5.7. 🙏

@ruudk
Copy link

ruudk commented Dec 12, 2018

@yguedidi Having the same issue, why isn't this patched in 5.6?

@ruudk
Copy link

ruudk commented Dec 12, 2018

Also, there is no way of using 5.x in Composer because it's not registered as a branch alias (see https://packagist.org/packages/facebook/graph-sdk)

@julienbourdeau
Copy link

Honestly, I'm not sure a release is coming soon, probably not at this time of the year.
I forked the repo and used my own branch for now, I'll get rid of this when 5.7 is out.

  1. fork repository
  2. update composer.json
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/julienbourdeau/php-graph-sdk"
        }
    ],
    "require": {
        "php": "^7.1.3",
        "facebook/graph-sdk": "5.x-dev"
    },
  1. delete vendor folder (just in case) rm -rf vendor/facebook
  2. Update with composer update

@ruudk
Copy link

ruudk commented Dec 13, 2018

@julienbourdeau FYI, No need for a fork, you can also do this:

"repositories": [{
            "type": "vcs",
            "url": "http://github.com/facebook/php-graph-sdk"
        }
],
    "require": {
        "php": "^7.1.3",
        "facebook/graph-sdk": "5.x-dev"
    },

@julienbourdeau
Copy link

julienbourdeau commented Dec 14, 2018

That's what I expected but for some reason, it didn't work. I didn't want waste too much time so I didn't look into the details ¯_(ツ)_/¯

@yguedidi
Copy link
Contributor

5.7.0 finally released! Sorry for the delay.. Thanks to who provide workarounds!

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

No branches or pull requests

8 participants