Catching error in DateTime::Format

[Anonymous Monk]

I'm an inexperienced Perl programmer (my background is mainly in Python), charged with keeping a legacy app running, and I'm having trouble figuring out how to validate a date. (At least, that's what I think the problem is.)

We have a function that takes a date value and does some transformations on it. This is thoroughly tested and works fine for its use case, but we don't actually have a test for what happens if someone sends garbage into it—it assumes the input really is a valid date. Normally, this is fine; the input comes from internal sources and should always be OK. We now need to extend it for a case when we don't have control over the incoming data. So, I tried to wrap it in an eval to catch any errors. Currently, it looks like this:

# in MyApp::Util
sub convert_datetime {
    my ( $self, $incoming_datetime ) = @_;

    my $new_dt;
    eval { $new_dt = DateTime::Format::ISO8601->parse_datetime( $incom
+ing_datetime ); };
    return undef if $@;
    # No error, now we can convert the date (not shown)
}

The test script now has this:

my $bogus_datetime = MyApp::Util->convert_datetime("FOO");
is ($bogus_datetime, undef, "convert_datetime doesn't blow up if sent 
+bogus date");

When I run this, however, I get (I've changed the file paths):

ok 141 - convert_datetime doesn't blow up if sent bogus date
Invalid date format:  at /path/to/MyApp/Util.pm line 818.
    eval {...} called at /path/to/MyApp/Util.pm line 818
    MyApp::Util::convert_datetime("MyApp::Util", "FOO") called at t/Ut
+il.t line 297
    ...propagated at t/Util.t line 298.

So the test "works" in that it returns an "ok" value, but it also prints the whole "Invalid date format" thing. How do I get it to _not_ do this, or otherwise test to see if the input is valid before I pass it to DateTime::Format::ISO8601->parse_datetime? The docs for this function say "The parse_datetime method will attempt to match these ambiguous strings" etc., but doesn't say what happens if it can't, or what you're supposed to do to test for valid input.

I tried to write a test script, but even though it looks identical (to me), this does _not_ print the "Invalid date format":

#!/usr/bin/env perl

use strict;
use warnings;

use DateTime::Format::ISO8601;
eval {my $foo = DateTime::Format::ISO8601->parse_datetime("FOO");};
print "ERROR\n" if $@;

Running this simply prints "ERROR" and that's it, no other output. What's going on in the real code, or what's the right way to handle this?

[NERDVANA]

Actually using $@ as a boolean is a bug waiting to happen. It relies on the exception being a non-empty non-zero string, and strange interactions between object destructors can sometimes break that assumption.

A safer pattern is

unless (eval { ...; 1 }) {
  ... # handle the error in $@
}

or for simpler cases,

eval { ...; 1 } or return undef;

return undef unless eval { ...; 1 };

Eval is guaranteed to return a false value on an exception, so you ensure your expression ends with a true value, and now you aren't at the mercy of the exception text/object.

[ysth]

Or

use 5.040;
try { .... } catch { return undef }

In 5.34 to 5.48, it's experimental and requires suppressing experimental warnings; using finally remains experimental; see https://perldoc.perl.org/perlsyn#Try-Catch-Exception-Handling.

[hippo]

use 5.040; ... In 5.34 to 5.[3]8 ...

Or just use Feature::Compat::Try and save any bother over which version of perl might be running it.

[ikegami]

5.48 should be 5.38

[bliako]

A simple die prints $@ of last failed eval, even without explicitly telling it to do so. And that's confusing if you decided to move on from the failed eval and then you throw a die and it talks about past things... Perhaps something else in the code or Test::More does similar?

use strict;
use warnings;
use DateTime;

my $x = eval{ DateTime->from_epoch(epoch=>'xxxx'); };
print "xx\n";
die;
# moving on to other business:
my $z = 1; #eval { 1 }; # "reset" $@
die; # still displays the $@ of last failed eval which is irrelevant a
+t this point in the code!

[Anonymous Monk]

OP here. Wow, yes, that was it! I put a "die" in there so I could study this one test while I was working on it. When I removed that, the tests worked as normal. And of course I didn't show the entire test suite in my example here, so I didn't include the "die" at the end of the relevant section.

Thank you so much! I never, ever would have figured that out. (In fact, it would have been even more confusing after I'd removed the "die" to run the complete test suite, and the test started working fine.)

[bliako]

hehe, die never moves on. Unless you reset $@ with reset('@'); prior to calling die, after your eval.

[ikegami]

Something is using $SIG{__DIE__} or $SIG{__WARN__}, or something is overriding Carp::croak or die, or something is printing $@, or ...

What's at t/Util.t line 298? That's from where the exception is thrown.

[Anonymous Monk]

t/Util.t line 298 is the "is ($bogus_datetime...." line in the "The test script now has this" in my original post.

The app is based on Catalyst, if that affects things. The test module (i.e. t/Util.t) doesn't use anything other than Test::More, though, and doesn't seem to be doing anything fancy; that is, it doesn't look to me like it's using $SIG or overriding other modules.

[ikegami]

No, the line had the die; you've since mentioned you had.

[Corion]

You want to catch the exception by using eval in your test script, like it does in the original code:

my $bogus_datetime = eval { MyApp::Util->convert_datetime("FOO"); };
is ($bogus_datetime, undef, "convert_datetime doesn't blow up if sent 
+bogus date");

Update: I misread - I don't know how to suppress the output. What test module is t/Util.t using?

[Anonymous Monk]

OP here—It's using Test::More.

Should I even be relying on eval at all? Or should I instead be checking it first, before I send it to DateTime::Format::ISO8601? Handling dates seems extremely complicated, and I don't know what the correct approach is. I'm also not sure where this error will go when running the actual code; if it's just an artifact of testing, I guess I can just comment out the test and not worry about it, if this output is going to vanish into thin air in real use.

[Corion]

I think the approach is sound.

The following script does not output the additional warnings, so it must be some other module that loads (say) Carp or something else that outputs these (caught) warnings.

use 5.020;

use Test::More;
use DateTime;

package MyApp::Util {
    # in MyApp::Util
    sub convert_datetime {
        my ( $self, $incoming_datetime ) = @_;

        my $new_dt;
        eval { $new_dt = DateTime::Format::ISO8601->parse_datetime( $i
+ncoming_datetime ); };
        return undef if $@;
        # No error, now we can convert the date (not shown)
        return $new_dt;
    }
}

my $bogus_datetime = MyApp::Util->convert_datetime("FOO");
is ($bogus_datetime, undef, "convert_datetime doesn't blow up if sent 
+bogus date");

done_testing;