petergoldstein / dalli

High performance memcached client for Ruby
MIT License
3.1k stars 450 forks source link

Serializer: reraise all .load errors as UnmarshalError #1011

Closed olleolleolle closed 2 weeks ago

olleolleolle commented 1 month ago

Doesn’t any error during deserialising mean the data is bad?

In order to avoid missing an error message to filter out, treat any Marshal.load error as a failed serialization, and trust Ruby's e.cause system (edit: introduced in Ruby 2.1) to provide a lineage of the error's true beginning.

This change was inspired by a comment relating to #1008.

With this rescue, it's possible to dismantle the other regular expressions.


This PR supersedes #1009, #1008

petergoldstein commented 1 month ago

@olleolleolle So is this an alternative to #1008 ?

Are there any concerns on how this works with older Ruby versions? Will they see a meaningful change in behavior?

petergoldstein commented 1 month ago

@olleolleolle It would helpful to understand how these different PRs relate, since they seem to touch similar code.

olleolleolle commented 1 month ago

@olleolleolle It would helpful to understand how these different PRs relate, since they seem to touch similar code.

Ah, thanks for asking, they're sort of the evolution of my thinking on this.

This is the more thorough change, relying on the Ruby feature of "cause" in exception handling.

The previous ones were about "not editing too much in the project", trying to use a light touch.