Php-graph-sdk: FacebookBatchResponse failed

[AidasK]

I was sending loads of successful batch requests when this happened:

...
 if (empty($requests)) {
                continue;
}
$a = $facebook->sendBatchRequest($requests)->getResponses();

 [Symfony\Component\Debug\Exception\FatalThrowableError]
  Type error: Argument 1 passed to Facebook\FacebookResponse::__construct() must be an instance of Facebook\FacebookRequest, null given, called in /var/www/versions/1468485597/vendor/facebook/php-sdk-v4/src/Facebook/FacebookBatchResponse.php on line 108

facebook/php-sdk-v4 5.2.0
php7.0

[SammyK]

Hey @AidasK! Can you include a bit more code that shows how you are creating the requests before you send them to sendBatchRequest()? :)

[yguedidi]

I think the case of timeouted batched request is not handled properly

[SammyK]

@yguedidi Ah! Good call. We'll need to look into this. I should be able to give some more TLC in August. :)

[AidasK]

I haven't included it, because it is quite straight forward. This error has happened to me only once. I have sent thousands of notifications before and only one time this error occurred.

$requests = [];
foreach ($ids as $id) {
                $requests[] = $facebook->request('post', "/$id/notifications", [
                    'template' => 'You have a new message',
                    'href' => '?p=' . urlencode($id),
                    'ref' => 'abc',
                ]);
}
 if (empty($requests)) {
                continue;
}
$a = $facebook->sendBatchRequest($requests)->getResponses();

[SammyK]

Thanks @AidasK! I think @yguedidi nailed it on the head - we need to do better handling of failed batch requests. :)

[yguedidi]

@SammyK I'll try ASAP

[yguedidi]

@AidasK can you please confirm that the bug happen when you have more that 50 items in your $ids array? Thanks

[AidasK]

I think I was sending exactly 50 notifications in one batch at a time. But not more, because of FB limitations.

[AidasK]

p.s. I don't use facebook notifications anymore, so I don't know if it was fixed. You can close it for now

[yguedidi]

@SammyK what do you think if we close this? We can't reproduce it, it's not the more than 50 requests (we already have a safeguard), and it only happened once to @AidasK.

[AidasK]

This error has appeared 4 times today:

Type error: Argument 1 passed to Facebook\FacebookResponse::__construct() must be an instance of Facebook\FacebookRequest, null given, called in /var/www/versions/1498736334/vendor/facebook/graph-sdk/src/Facebook/FacebookBatchResponse.php on line 108

This is part of my stacktrace:

It seems that facebook has responded with an error page, but sdk can't handle it correctly.

facebook/graph-sdk 5.5.0

[SammyK]

Hum... I'm curious how we could reproduce this.

[AidasK]

I don't think we can reproduce it, it is an error from facebook api, but sdk should handle it

[saulirajala]

I agree with @AidasK. The same issue happened today when Facebook API had some technical problems and was down for a while at least here in Finland. Because of unresponsiveness of API, the first argument for constructor of class FacebookResponse was null, which caused fatal error.

Event though these cases are edge cases, I think SDK should be able to survive from it without fatal error. I'm not that familiar with the architecture of SDK, but @SammyK solution in #636 seems fine or maybe even something like this could work:

$originalRequest = isset($this->batchRequest[$key]['request']) ? $this->batchRequest[$key]['request'] : null;

if ( ! $originalRequest ) {
    return;
}

[yguedidi]

For me, the best fix is to allow null for the original request, but this is a BC break.. I'll then accept the solution proposed in #636 (with some changes)

[BinRoq]

This error has appeared 4 times today:

Type error: Argument 1 passed to Facebook\FacebookResponse::__construct() must be an instance of Facebook\FacebookRequest, null given, called in /var/www/versions/1498736334/vendor/facebook/graph-sdk/src/Facebook/FacebookBatchResponse.php on line 108
This is part of my stacktrace:
screen shot 2017-06-29 at 15 05 03
screen shot 2017-06-29 at 15 08 16

It seems that facebook has responded with an error page, but sdk can't handle it correctly.

facebook/graph-sdk 5.5.0