diff options
author | Tom Ryder <tom@sanctum.geek.nz> | 2018-05-24 13:01:21 +1200 |
---|---|---|
committer | Tom Ryder <tom@sanctum.geek.nz> | 2018-05-24 13:01:21 +1200 |
commit | 301a3564e6b7cf716b75440c5333e688ea814873 (patch) | |
tree | 2fc66a50614dffea3c5aefdedddf2075f7802c81 | |
parent | 8180da509371afd3f3044b494a8cb87e70e73d64 (diff) | |
download | monitoring-plugin-perl-301a3564e6b7cf716b75440c5333e688ea814873.tar.gz |
Fail with config ->errstr() when undef read
The Config::Tiny parent class for Monitoring::Plugin::Config does not
necessarily raise an exception in $@/$EVAL_ERROR on a failed call to its
->read() method. Indeed, it does not in most cases, at least in the most
recent Config::Tiny.
If a file does not exist or for whatever other reason cannot actually be
read--such as permissions problems--Monitoring::Plugin ends up reporting
a "Missing config section" to the caller, which is misleading.
This information is available from the ->errstr() method of the base
class, however, and an inspection of the code for the ->read() method in
the parent class suggests the correct approach is to look for a return
of `undef` from the ->read() method, and use the return value of
->errstr() as the reported error for ->_die(). This commit adds such a
check.
To reproduce, given the following plugin `mptest`:
use strict;
use warnings;
use Monitoring::Plugin qw(%ERRORS);
my $mp = Monitoring::Plugin->new(
usage => '',
);
$mp->getopts;
$mp->plugin_exit($ERRORS{OK}, 'Done!');
Running it without options of course works, like so:
$ perl mptest
MPTEST OK - Done!
However, prior to this patch, if we specify --extra-opts with a
nonexistent filename, we get a misleading error:
$ perl mptest --extra-opts=@/nonexistent
Invalid section 'mptest' in config file '/nonexistent'
With this patch included, the error is more accurate and helpful:
$ perl mptest --extra-opts=@/nonexistent
Failed to open file '/nonexistent' for reading: No such file or directory
-rw-r--r-- | lib/Monitoring/Plugin/Getopt.pm | 4 |
1 files changed, 3 insertions, 1 deletions
diff --git a/lib/Monitoring/Plugin/Getopt.pm b/lib/Monitoring/Plugin/Getopt.pm index c8033d0..1810fde 100644 --- a/lib/Monitoring/Plugin/Getopt.pm +++ b/lib/Monitoring/Plugin/Getopt.pm | |||
@@ -260,7 +260,9 @@ sub _load_config_section | |||
260 | 260 | ||
261 | my $Config; | 261 | my $Config; |
262 | eval { $Config = Monitoring::Plugin::Config->read($file); }; | 262 | eval { $Config = Monitoring::Plugin::Config->read($file); }; |
263 | $self->_die($@) if ($@); #TODO: add test? | 263 | $self->_die($@) if ($@); |
264 | defined $Config | ||
265 | or $self->_die(Monitoring::Plugin::Config->errstr); | ||
264 | 266 | ||
265 | # TODO: is this check sane? Does --extra-opts=foo require a [foo] section? | 267 | # TODO: is this check sane? Does --extra-opts=foo require a [foo] section? |
266 | ## Nevertheless, if we die as UNKNOWN here we should do the same on default | 268 | ## Nevertheless, if we die as UNKNOWN here we should do the same on default |