Subject: | defend against sigpipe in Firefox::Marionette |
Date: | Fri, 13 Dec 2019 13:21:08 -0700 |
To: | bug-firefox-marionette [...] rt.cpan.org |
From: | John Denker <jsd [...] av8n.com> |
Hi --
Desired behavior:
*) Library modules should never terminate the process. If
need be, they can throw exceptions, but these should be
catchable.
*) If an operation produces an error message, repeating the
operation should produce the same error message.
Observed behavior:
If the browser quits, then:
1) The next Marionette operation will throw an exception
(((Firefox killed by a USR1 signal (10) in my/script at line 1002)))
This is good behavior. The exception can be caught with
eval {....} in the usual way.
2) The operation after that causes the entire process to
be terminated by a SIGPIPE.
Plain eval {....} does not catch this.
This is a royal pain. The penalty for operating on a dead
browser should not be losing all your work.
===========
*) As a workaround, the user could catch sigpipe.
$SIG{PIPE} = sub { Carp::croak "Caught a sigpipe: $!" };
However, this is not mentioned in the Firefox::Marionette
documentation, and it unnecessarily increases user workload.
It is not consistent with the overall design of the package.
*) As an even worse workaround, the user could guard every
Marionette operation with $driver->alive(), but that is
even more workload for the user.
========================================================
SPECIFIC CONSTRUCTIVE SUGGESTION: Defend against sigpipe.
The following patch should be more-or-less self-explanatory.
It makes things more consistent and more convenient for
everybody.
In particular, it means repeated operations all produce
the same error message.
--- Marionette.pm#1 2019-12-13 02:32:45.949508275 -0700
+++ Marionette.pm 2019-12-13 04:18:28.078483824 -0700
@@ -29,6 +29,7 @@
use Carp();
use Config;
use base qw(Exporter);
+use Scalar::Util qw(openhandle);
BEGIN {
if ( $OSNAME eq 'MSWin32' ) {
@@ -1582,6 +1583,10 @@
}
}
}
+# Close the handle, because writing to a broken pipe is fatal,
+# but writing to a closed handle is not. Also, openhandle()
+# detects closed, but does not detect broken.
+ close($self->{_socket}) if defined $self->{_child_error};
return;
}
@@ -4600,7 +4605,11 @@
if ( $self->_debug() ) {
warn ">> $length:$json\n";
}
- my $result = syswrite $self->_socket(), "$length:$json";
+ my $result;
+# Validate the handle, to avoid an annoying message on STDERR:
+ if (openhandle $self->_socket()) {
+ $result = syswrite $self->_socket(), "$length:$json";
+ }
if ( !defined $result ) {
my $socket_error = $EXTENDED_OS_ERROR;
if ( $self->alive() ) {