Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore :undefined_variable "reason" in getvar #665

Merged
merged 1 commit into from Oct 8, 2016

Conversation

mks-m
Copy link
Contributor

@mks-m mks-m commented Oct 6, 2016

catch returns value of second argument to throw, which until puppetlabs/puppet@860a276#diff-bb62983f077c2b43d065be52de182209R514 was nil, but now is non-falsey reason for error. short-circuit using return and eval to nil if throw was caught.

`catch` returns value of second argument to `throw`, which until 860a2761f334c964068038b3ef6853f08beb1df5 was `nil`, but now is non-falsey reason for error. short-circuit using return and eval to nil if `throw` was caught.
@DavidS
Copy link
Contributor

DavidS commented Oct 8, 2016

I could reproduce this on the quick with puppet 4.7.0 only by modifying the puppet source code (see below). Still, the code is more robust with this change so in it goes!

diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 57294c1..a1fb5f0 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -506,7 +506,7 @@ class Puppet::Parser::Scope
       if next_scope
         next_scope.lookupvar(name, options)
       else
-        variable_not_found(name)
+        variable_not_found(name, "error")
       end
     end
   end

@DavidS DavidS merged commit f6bd01b into puppetlabs:master Oct 8, 2016
@mks-m
Copy link
Contributor Author

mks-m commented Oct 8, 2016

@DavidS what tripped me was a mis-used variable name: getvar('$::some::var') which raised exception "class $ not found" which then leaked into result of getvar. thanks for merge.

DavidS added a commit to DavidS/puppetlabs-stdlib that referenced this pull request Oct 8, 2016
@keymone added information on how to reproduce his issue, so here's the
test case that would fail without his change.
@DavidS
Copy link
Contributor

DavidS commented Oct 8, 2016

@keymone See DavidS@ba67577 . thanks for coming back!

HelenCampbell pushed a commit that referenced this pull request Oct 10, 2016
@keymone added information on how to reproduce his issue, so here's the
test case that would fail without his change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants