Add fix for #619 #636
Add fix for #619 #636
Conversation
ghost
added
the
|
This looks weird to me... I'm 👎 on this solution The error means that we receive a response that does not have a corresponding request (or at least a corresponding response we do not found). I think it is better to decide what to do in this case:
Any preference? :) |
|
I'm cool with throwing an exception. But in what scenario would that happen? I tried to trigger the error by manually throwing invalid values at it but I couldn't trigger it! :) |
|
OMG!!! @SammyK I think I got it! Not tried yet (late night idea), but I think this can happen due to the limit of 50 requests per batch request. 😄 |
|
That's so awesome! Nice job @yguedidi! And thanks for knocking out some of these issues! I've been pretty swamped lately so I'm looking forward to getting some time to work on the SDK when things let up! :) |
| @@ -98,7 +98,7 @@ public function setResponses(array $responses) | |||
| public function addResponse($key, $response) | |||
| { | |||
| $originalRequestName = isset($this->batchRequest[$key]['name']) ? $this->batchRequest[$key]['name'] : $key; | |||
| $originalRequest = isset($this->batchRequest[$key]['request']) ? $this->batchRequest[$key]['request'] : null; | |||
| $originalRequest = isset($this->batchRequest[$key]['request']) ? $this->batchRequest[$key]['request'] : new FacebookRequest; | |||
There was a problem hiding this comment.
Instead of a FacebookRequest, I'd use an UnknownFacebookRequest which extends FacebookRequest.
That way, we don't break BC, and the user can check with a simple instanceof if the original request is there or not.
I've tried my darnedest to replicate the issue in #619, but I can't replicate it for the life of me. I was thinking it was because of timeouts where the response will be
null, but it handlesnullresponses just fine.I'm thinking this has to be an extreme edge case. At any rate, I added this little patch to keep error messages from being generated if the edge case happens again.
@yguedidi - what do you think? :)