Skip to content
This repository has been archived by the owner on Jun 27, 2022. It is now read-only.

Update CachePlugin.php #44

Closed
wants to merge 1 commit into from
Closed

Update CachePlugin.php #44

wants to merge 1 commit into from

Conversation

gena01
Copy link

@gena01 gena01 commented Oct 22, 2014

Fixing a try... catch block to catch RequestException(s). This fixes the Batch calls for us, otherwise the batch service aborts prematurely since the exception bubbles up to the caller and the rest of the requests are not being processed.

Fixing a try... catch block to catch RequestException(s). This fixes the Batch calls for us, otherwise the batch service aborts prematurely since the exception bubbles up to the caller and the rest of the requests are not being processed.
@gena01
Copy link
Author

gena01 commented Oct 27, 2014

@mtdowling not sure if I should fix the unit test or open an issue on this.

Ran into this when trying to use Batch Service functionality in combination with CachePlugin. I have 2 requests:

  1. returns a 200 Json response.
  2. Returns a 404.
    With cacheplugin running it tries to cache both responses it seems and then on revalidate misses the catch for BadResponseException which bubbles up to the caller and aborts the batch sending. (losing the other requests in the batch).

So it's either me doing something wrong or CachePlugin needs to handle things better. Any ideas?

@gena01
Copy link
Author

gena01 commented Dec 2, 2014

@mtdowling can you take a quick look at this? Thank you.

@mtdowling
Copy link
Member

A 404 in response to a validation request means that the thing you previously cached is no longer there. In this case, I think the exception should be thrown. Right now it only catches CurlExceptions which means that it will use the cached response. I think this is the right thing to do based on http://tools.ietf.org/html/rfc7234#section-4.3.3

@gena01
Copy link
Author

gena01 commented Dec 2, 2014

@mtdowling my problem is that if I am using BatchService I lose the other responses because of the Exception bubbling up...

@mtdowling
Copy link
Member

Hm. Sounds like a bug in the batch abstraction not necessarily the cache subscriber. There are other known issues in the batching abstraction that have been addressed in subsequent major versions of Guzzle. I will probably never have time to address this in Guzzle 3, but I'm open for pull requests to fix the batch abstraction.

@mtdowling mtdowling closed this Dec 18, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants