fennb / phirehose

PHP interface to Twitter Streaming API
706 stars 189 forks source link

Undefined Index: Phirehose 688 #81

Closed Schenn closed 3 years ago

Schenn commented 9 years ago

When running the script that grabs tweets from the stream, Phirehose reliably throws at line 688.

Sometimes its index 2, sometimes it's index 3.

When I was stepping through the consume process:

The http response from the server was "http1.1 200 ok" but xdebug says that the httpCode variable was set to "close" instead of 200. Then at line 688, it's attempting to parse a whitespace (/r/n) and throws the undefined index.

zabojad commented 8 years ago

I have the exact same issue on line 689 of this very same file. Any hint to solve this ?

DarrenCook commented 8 years ago

https://github.com/fennb/phirehose/blob/master/lib/Phirehose.php#L689

I.e. it expecting the first line of the response to be "HTTP/1.1 200 OK", so it is assuming exactly two spaces. If you get a PHP error on that line, something is screwed up: the server is sending data that it should never send. From Schenn's comment I guess the server has sent a "Close" message.

(I wonder - are either of you going through a proxy?)

As that line is never going to be a performance bottleneck, let's make it more robust. Replace the current line 689 with this:

$statusResponse = fgets($this->conn, 1024);
$statusResponseParts = preg_split('/\s+/', trim($statusResponse), 3);
if(count($statusResponseParts) != 3){
  $httpVer = "<bad>";
  $httpCode = "<bad>";
  $httpMessage = $statusResponse;
  }
else list($httpVer, $httpCode, $httpMessage) = $statusResponseParts;

I've not tested that, but it should now treat it as any other connection error, and log what the exact message is that you get.

(If you try it, and it works, let us know, and someone can do a patch.)

Schenn commented 8 years ago

No, twitter is sending keep alives which are empty strings at this point. On Oct 15, 2015 9:24 AM, "Darren Cook" notifications@github.com wrote:

https://github.com/fennb/phirehose/blob/master/lib/Phirehose.php#L689

I.e. it expecting the first line of the response to be "HTTP/1.1 200 OK", so it is assuming exactly two spaces. If you get a PHP error on that line, something is screwed up: the server is sending data that it should never send. From Schenn's comment I guess the server has sent a "Close" message.

(I wonder - are either of you going through a proxy?)

As that line is never going to be a performance bottleneck, let's make it more robust. Replace the current line 689 with this:

$statusResponse = fgets($this->conn, 1024); $statusResponseParts = preg_split('/\s+/', trim($statusResponse), 3); if(count($statusResponseParts) != 3){ $httpVer = ""; $httpCode = ""; $httpMessage = $statusResponse; } else list($httpVer, $httpCode, $httpMessage) = $statusResponseParts;

I've not tested that, but it should now treat it as any other connection error, and log what the exact message is that you get.

(If you try it, and it works, let us know, and someone can do a patch.)

— Reply to this email directly or view it on GitHub https://github.com/fennb/phirehose/issues/81#issuecomment-148443841.

DarrenCook commented 8 years ago

@Schenn what was the "no" referring to?

Line 689 is in the connect() function, and is setting up the initial http connection. It is only called once in the lifetime of the script. Keep-alives from Twitter cannot be involved.

Schenn commented 8 years ago

I tried stepping through it before, but it's been a few months so forgive the lack of details. By the time the code failed at this point, the string was empty. Doing a little research showed that twitter now sends keep alives of empty strings. Since the string is empty, it cant be split into an array so the referenced index position doesn't exist. On Oct 15, 2015 10:07 AM, "Darren Cook" notifications@github.com wrote:

@Schenn https://github.com/Schenn what was the "no" referring to?

Line 689 is in the connect() function, and is setting up the initial http connection. It is only called once in the lifetime of the script. Keep-alives from Twitter cannot be involved.

— Reply to this email directly or view it on GitHub https://github.com/fennb/phirehose/issues/81#issuecomment-148459221.

Schenn commented 8 years ago

My solution was to use the beta of codebird 3.0 coming out in December, so thats not much help. Sorry. On Oct 15, 2015 10:44 AM, "Schenn" schenn@gmail.com wrote:

I tried stepping through it before, but it's been a few months so forgive the lack of details. By the time the code failed at this point, the string was empty. Doing a little research showed that twitter now sends keep alives of empty strings. Since the string is empty, it cant be split into an array so the referenced index position doesn't exist. On Oct 15, 2015 10:07 AM, "Darren Cook" notifications@github.com wrote:

@Schenn https://github.com/Schenn what was the "no" referring to?

Line 689 is in the connect() function, and is setting up the initial http connection. It is only called once in the lifetime of the script. Keep-alives from Twitter cannot be involved.

— Reply to this email directly or view it on GitHub https://github.com/fennb/phirehose/issues/81#issuecomment-148459221.

nicklee1990 commented 8 years ago

$statusResponse = fgets($this->conn, 1024); $statusResponseParts = preg_split('/\s+/', trim($statusResponse), 3); if(count($statusResponseParts) != 3){ $httpVer = ""; $httpCode = ""; $httpMessage = $statusResponse; } else list($httpVer, $httpCode, $httpMessage) = $statusResponseParts;

this code appears to be working for me. I was seeing this error due to the Twitter Service being temporarily unavailable: HTTP ERROR 503: Service Temporarily Unavailable ().

fennb commented 8 years ago

This is really helpful. @nicklee1990 Do you mind posting the exact value of $statusResponse as per your code above, so we can come up with a robust solution?

Thanks!

zabojad commented 8 years ago

Just for the record, I was seeing this error message for the exact same reasons as @nicklee1990... I think I was experiencing this besause as was using the same twitter app ids and tokens in two different places at the same time. In that case, Twitter closes the first connection to keep only one (the latest) going...

Schenn commented 8 years ago

This could have been happening with me as well. Stepping through, I would get to teitters 420 error within minutes On Oct 17, 2015 1:05 AM, "Thomas Fétiveau" notifications@github.com wrote:

Just for the record, I was seeing this error message for the exact same reasons as @nicklee1990 https://github.com/nicklee1990... I think I was experiencing this besause as was using the same twitter app ids and tokens in two different places at the same time. In that case, Twitter closes the first connection to keep only one (the latest) going...

— Reply to this email directly or view it on GitHub https://github.com/fennb/phirehose/issues/81#issuecomment-148894152.