Merge remote-tracking branch 'marco/invalid_timeout_reason_issue35'

p-alik · Jan 3, 2018 · 9091978 · 9091978
2 parents e063807 + 8e5c0aa
commit 9091978
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 6 deletions.
diff --git a/lib/Gearman/Taskset.pm b/lib/Gearman/Taskset.pm
@@ -211,10 +211,16 @@ everything has finished running or failing.
 
 sub wait {
     my ($self, %opts) = @_;
-    my $timeout;
+    my ($timeout, $given_timeout_s);
     if (exists $opts{timeout}) {
         $timeout = delete $opts{timeout};
-        $timeout += Time::HiRes::time() if defined $timeout;
+        if (defined $timeout) {
+            ## keep the given timeout value for the failure reason
+            #  Handles issue #35
+            #  https://github.com/p-alik/perl-Gearman/issues/35
+            $given_timeout_s = $timeout;
+            $timeout += Time::HiRes::time();
+        }
     }
 
     Carp::carp "Unknown options: "
@@ -270,7 +276,10 @@ sub wait {
             for (values %{ $self->{waiting} }) {
                 for (@$_) {
                     my $func = $_->func;
-                    $_->fail("Task $func elapsed timeout [${timeout}s]");
+                    ## use the given timeout here
+                    #  Handles issue #35
+                    #  https://github.com/p-alik/perl-Gearman/issues/35
+                    $_->fail("Task $func elapsed timeout [${given_timeout_s}s]");
                 }
             } ## end for (values %{ $self->{...}})
             $self->cancel;

diff --git a/t/02-client.t b/t/02-client.t
@@ -74,7 +74,7 @@ subtest 'Client: "on_fail" handler is triggered on timeout' => sub {
     my @job_servers = $gts->job_servers();
     @job_servers || plan skip_all => $t::Server::ERROR;
 
-    my ($now, $then)   = (time);
+    my ($reason, $now, $then) = (q(NO REASON), time);
     my $c              = new_ok($mn, [job_servers => [@job_servers]]);
     my $timeout        = 2;
     my $initial_error  = '"on_fail" was NOT triggered';
@@ -83,8 +83,13 @@ subtest 'Client: "on_fail" handler is triggered on timeout' => sub {
     my $res_ref        = $c->do_task(
         task_that_does_not_exist => '',
         {
-            timeout     => $timeout,
-            on_fail     => sub { $then = time; $error = $expected_error },
+            timeout => $timeout,
+            on_fail => sub {
+                ## keep the given reason
+                ($reason) = @_;
+                $then  = time;
+                $error = $expected_error;
+            },
             on_complete => sub {
                 die '"on_complete" handler was called unexpectedly';
             },
@@ -97,6 +102,23 @@ subtest 'Client: "on_fail" handler is triggered on timeout' => sub {
         "Timeout of ${timeout}s was elapsed"
     );
 
+    # check if reason was set as expected
+    # fixes unwanted behaviour:
+    #
+    #   Task task_that_does_not_exist elapsed timeout [1514988291.35953s]
+    #
+    # Should be (e.g.):
+    #
+    #   Task task_that_does_not_exist elapsed timeout [2s]
+    #
+    # Proves fix of issue #35
+    # https://github.com/p-alik/perl-Gearman/issues/35
+    like(
+        $reason,
+        qr/Task [^[:space:]]+ elapsed timeout \[${timeout}s\]/,
+        q(Failure reason contains given timeout value)
+    );
+
     $expected_error = qq(ALRM handler fired after ${timeout}s);
     throws_ok {
         local $SIG{ALRM} = sub { die $expected_error };