summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTom Ryder <tom@sanctum.geek.nz>2018-05-24 01:01:21 (GMT)
committerTom Ryder <tom@sanctum.geek.nz>2018-05-24 01:01:21 (GMT)
commit301a3564e6b7cf716b75440c5333e688ea814873 (patch)
tree2fc66a50614dffea3c5aefdedddf2075f7802c81
parent8180da509371afd3f3044b494a8cb87e70e73d64 (diff)
downloadmonitoring-plugin-perl-301a356.tar.gz
Fail with config ->errstr() when undef readrefs/pull/19/head
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.pm4
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