Skip to content

Conversation

@rvanlaak
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #41685
License MIT
Doc PR symfony/symfony-docs#...
  • introduces DebugAdapter to throw exception on failing save
  • adds cache.exception_on_save as parameter to configure the feature

@nicolas-grekas could you provide some initial feedback on whether this implementation is in the right direction?

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@rvanlaak rvanlaak marked this pull request as ready for review July 23, 2021 13:50
@carsonbot carsonbot added Status: Needs Review Cache Feature RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Jul 23, 2021
@carsonbot carsonbot changed the title [RFC][Cache] Allow decorating each cache.pool as debug adapters [Cache] Allow decorating each cache.pool as debug adapters Jul 23, 2021
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jul 23, 2021

That's an interesting approach, but to be honest I had another one in mind:
given that all errors end up being logged in CacheItem::log(), we could inject a throwing logger in debug mode instead.

That would give better DX, by giving proper error messages.

@nicolas-grekas
Copy link
Member

Closing in favor of #43148
Thanks for submitting!

nicolas-grekas added a commit that referenced this pull request Sep 24, 2021
…ion fails (nicolas-grekas)

This PR was merged into the 5.4 branch.

Discussion
----------

[Cache] Throw ValueError in debug mode when serialization fails

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | -
| Tickets       | Fix #41685
| License       | MIT
| Doc PR        | -

This feature allows spotting mistakes at the dev stage when trying to store unserializable objects (typically a closure) into a cache.

This PR replaces #42241 with a simpler and more focused approach: here we fail via a simple `ValueError`, which won't get caught by any try/catch into the Cache component, and we fail only when serialization fails - not on any failures of cache pool methods.

Commits
-------

0d3ede7 [Cache] Throw ValueError in debug mode when serialization fails
@rvanlaak rvanlaak deleted the cache-debug-exceptions branch September 25, 2021 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cache Feature RFC RFC = Request For Comments (proposals about features that you want to be discussed) Status: Needs Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In debug mode, the cache component could throw when save() fails

3 participants