Fix cpantesters FAILs: gen.t perl mismatch + class-method serializer crash

[bigpresh]

Two fixes for v2.2.1 cpantesters failures

1. t/e2e/cli/gen.t: use TAP::Harness instead of shelling out to prove

The generated-app subtest ran system(q{prove}, q{-lr}, q{t}), which lets PATH decide which perl binary executes prove. On smokers with multiple perls installed, the one on PATH can differ from the one running the test suite, producing either:

• Can't locate Module::Runtime.pm in @INC (wrong @inc — e.g. ANDK report 24b36498)
• XS handshake key mismatch (wrong perl entirely — e.g. report 0893fc30)

Replace the system() call with TAP::Harness, which invokes child perls via $^X (the current interpreter) by construction. This eliminates the entire class of PATH/shell mismatch bugs rather than guarding against each one. TAP::Harness is core since 5.10.1; Dancer2 requires 5.12.0.

This test was newly added in v2.2.0, which is why this failure is new.

2. Role::Serializer + JSON/YAML helpers: fix class-method crash (GH #1837)

from_json/from_yaml/to_json/to_yaml called deserialize/serialize as class methods (__PACKAGE__->deserialize(@_)). When the underlying parser dies (e.g. on malformed input), the around deserialize modifier in Role::Serializer tried to call $self->log_cb — but $self is a package-name string on the class-method path, not an object, so Class::XSAccessor raises:

Class::XSAccessor: invalid instance method invocant: no hash ref supplied

This completely masks the real parse error. The sibling around serialize modifier already guards every $self-> call with blessed $self && — around deserialize simply missed the same treatment.

Two changes:

1. Role::Serializer: guard the log_cb call with blessed $self, matching the pattern already used in around serialize.
2. JSON/YAML helpers: construct a default instance (__PACKAGE__->new(log_cb => sub {})) so that deserialize/serialize always receive a proper object, allowing errors to be logged rather than swallowed.

Note: this bug does not cause any v2.2.1 test failures on cpantesters (the yaml.t test already works around it by calling on an instance), but it bites any application code that calls from_json($bad_input) / from_yaml($bad_input) directly.

Fixes GH #1837.

[bigpreshkt]

The execution of the generated app's tests via a system() call to prove still doesn't sit right with me. I'm going to look at using TAP::Harness instead.

[manwar]

@bigpresh Great work and thanks for prompt action.

[veryrusty]

Do we need to supply log_cb rather than use the default from Role::Serializer ( which is sub {1} ) ?

[bigpresh]

@veryrusty good catch, and it turned out to be a more interesting question than it looked.

You're right that log_cb => sub {} was pointless: Dancer2::Core::Role::Serializer already defaults it to sub { sub {1} }, so passing a no-op did nothing the default wasn't doing. But taking it out made me check what the default means for these helpers, and that uncovered something my earlier commit changed by accident.

Dancer2::Serializer::JSON::_invalid_utf8 deliberately branches on whether it has an object:

if ( blessed($self) ) {
    $self->log_cb->( warning => "$msg; leaving bytes unchanged" );
} else {
    warn "$msg; leaving bytes unchanged\n";
}

So by switching the helpers from __PACKAGE__->serialize(...) to __PACKAGE__->new(...)->serialize(...), I moved that notice out of the warn branch and into a logger that discards it:

to_json({ bad => "\xFF" })	result
main (class method, unblessed)	warns "Invalid UTF-8 in JSON data; leaving bytes unchanged"
this branch as it stood (->new, log_cb => sub {})	silent
->new with no log_cb at all (role default sub {1})	silent

Nothing in the suite covered that path for the helpers, which is why it went unnoticed. Two ways to settle it: accept the silence and treat _invalid_utf8's warn branch as dead code for these functions, or give the helpers a log_cb that actually reports. A change in behaviour like that should be a conscious decision rather than a quiet side effect, so I've gone with the second — pushed as b2e28ae:

sub _warn_log_cb {
    my ( $level, $message ) = @_;
    $message =~ s/\s+\z//;
    warn "$message\n";
    return 1;
}

sub from_json { __PACKAGE__->new( log_cb => \&_warn_log_cb )->deserialize(@_) }

That restores the UTF-8 notice, and as a bonus it delivers the thing #1837 actually complained about — the real parse error no longer disappears:

from_json('{not json')
  before:  dies with "Class::XSAccessor: invalid instance method invocant"
  as pushed earlier:  returns undef, silently
  now:  returns undef and warns
        "Failed to deserialize content: '"' expected, at character offset 1 (before "not json")"

New t/serializer_helpers.t pins both halves: the real error is reported for from_json/from_yaml, the UTF-8 notice still reaches STDERR, successful round trips stay quiet, and a bare Class->deserialize still fails silently without raising Class::XSAccessor (that last one covers the blessed $self guard in the role, which is what protects callers who don't go through these helpers). I checked the tests fail if either fix is reverted — reverting the role guard fails the class-method subtest, restoring sub {} fails three of the others.

The ->new does cost ~3× per call on small payloads (90k/s vs 300k/s class-method in a quick Benchmark run), which seems a fair price for one call per request.

Also pushed on the same branch: bbefda3 fixes the gen.t failures @gregoa reported in #1843. Different cause from the perl-mismatch ones — Debian patches the CPAN version check out of Dancer2::CLI::Gen for privacy (no-phone-home.patch) and the -x option that skips it goes with it, so the -x this test passes unconditionally aborts the generator with Unknown option: x before it writes anything. It now probes gen --help once and only passes -x where it exists; where the option is gone, so is the network access it exists to suppress. Verified with that patch applied to the tree.

[veryrusty]

Thanks for chasing down those log_cb's @bigpresh ! 👍 looks good.

[cromedome]

👍 Thanks, @bigpresh! Merged.

[gregoa]

Thanks alot for the change in bbefda3, @bigpresh!