Role::Serializer: class-method deserialize failure raises Class::XSAccessor, masking the real error

[bigpresh]

Status: confirmed against v2.2.0

Dancer2::Core::Role::Serializer's around deserialize calls $self->log_cb unconditionally in its error handler:

around deserialize => sub {
    my ( $orig, $self, $content, $options ) = @_;
    ...
    } or do {
        my $error = $@ || 'Zombie Error';
        $self->log_cb->( core => "Failed to deserialize content: $error" );   # line 74
    };

On the class-method call path $self is a package-name string, not an object, so the accessor blows up — and because this happens inside the error handler, the secondary error replaces the real one.

Reproduction

Both of these are documented public DSL keywords:

perl -Ilib -e '
use Dancer2::Serializer::JSON; use Dancer2::Serializer::YAML;
for my $t ( ["from_json", sub { Dancer2::Serializer::JSON::from_json("{not json") }],
            ["from_yaml", sub { Dancer2::Serializer::YAML::from_yaml("\tbad:\n  - [unclosed") }] ) {
  my ($name,$code) = @$t;
  my $r = eval { $code->() };
  printf "%-10s => %s\n", $name, ($@ ? "DIED: ".(split /\n/, $@)[0] : "returned ".(defined $r ? $r : "undef"));
}'

from_json  => DIED: Class::XSAccessor: invalid instance method invocant: no hash ref supplied at lib/Dancer2/Core/Role/Serializer.pm line 74.
from_yaml  => DIED: Class::XSAccessor: invalid instance method invocant: no hash ref supplied at lib/Dancer2/Core/Role/Serializer.pm line 74.

The actual JSON/YAML parse error is completely lost.

Why this is clearly a bug, not a design choice

The sibling around serialize in the same file gets this right — every $self-> call in it is guarded:

blessed $self && $self->execute_hook( 'engine.serializer.before', $content );
...
if ( blessed($self) && $self->config->{strict_utf8} ) { die $error }
blessed $self
    and $self->log_cb->( core => "Failed to serialize content: $error" );

deserialize simply missed the same treatment. The two modifiers should be symmetrical.

Impact

• from_json / from_yaml on malformed input die with an internal Class::XSAccessor message instead of reporting the parse failure. Very confusing to debug — the reported file and line point at Dancer2 internals, not at the caller or the bad data.
• Any code doing eval { from_json($untrusted) } gets a useless error string.
• The class-method path is not exotic: the DSL helpers (Dancer2::Serializer::JSON::from_json etc.) all call __PACKAGE__->deserialize(@_).

Suggested fix

Mirror the guard serialize already uses:

    } or do {
        my $error = $@ || 'Zombie Error';
        blessed $self
            and $self->log_cb->( core => "Failed to deserialize content: $error" );
    };

Worth considering whether a class-method failure should stay silent once the accessor no longer dies. serialize currently swallows it, so matching that is the consistent choice — but re-dieing the original $error when there is no log_cb to receive it would arguably serve callers better than returning undef. Either is an improvement on raising an unrelated error; pick one deliberately and document it.

Suggested test

Dancer2::Serializer::JSON->deserialize('{not json') called as a class method reports the JSON parse error (or returns undef quietly), and does not raise Class::XSAccessor.

How this surfaced

Found while verifying that the v2.2.0 Dumper removal (#1821) closed the reachability of the old text/x-data-dumper path. It is not a regression from that work — the missing guard predates it. Removing Dumper simply routes that content type to the JSON fallback, which is what made a class-method deserialize failure easy to trigger.

[bigpresh]

Confirmed by history — this is not a 2.2.0 regression. It has been present since v0.200000 (2016).

The asymmetry was introduced by two commits a day apart:

Commit	Date	Change
f6574f76	2016-05-30	"Reword logged error if serializaiton fails" — added the blessed $self guard to around serialize
f8a05e41	2016-05-31	"Reword logged error if deserializaiton fails" — same rewording for around deserialize, but without the guard

Both first shipped in v0.200000. The deserialize side simply did not pick up the guard its sibling had got the previous day, and nothing has touched it since.

For completeness: git log 21bc21d9..v2.2.0 -- lib/Dancer2/Core/Role/Serializer.pm is empty — the 2.2.0 release did not modify this file at all. The Dumper removal only made the latent bug easy to reach, by routing text/x-data-dumper to the JSON fallback where a class-method deserialize can now fail on ordinary input.

So: latent for nearly ten years, and the fix is a one-line guard.