+1, but I'm curious - what would a more general solution look like?

On Oct 25, 2009, at 4:47 PM, Markus Roberts wrote:

>
> If setup code for a process depends on network connectivity
> it needs to be protected with a rescue clause as much as the
> main body of the process.
>
> Further, Timeout exceptions aren't under StandardError and thus
> aren't caught by an un-typed rescue clause.  This doesn't matter
> if we've morphed the exception, but will cause the program to
> fail if we haven't.
>
> There are many places where these concerns _might_ cause a problem
> but in most cases they never will in practice; this patch addesses
> the two cases where I have been able to confirm that it actually
> can cause the client daemon to exit and two more where I suspect
> (but can not prove) that it could.
>
> I'd be willing to push this patch as it stands, as it at least
> fixes demonstrable problems.  A more general solution would be
> nice.
>
> Signed-off-by: Markus Roberts <[email protected]>
> ---
> lib/puppet/configurer.rb              |    7 ++++++-
> lib/puppet/indirector/facts/facter.rb |    2 +-
> lib/puppet/indirector/ldap.rb         |    4 ++--
> lib/puppet/ssl/host.rb                |    5 ++---
> 4 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
> index efda545..afe56c7 100644
> --- a/lib/puppet/configurer.rb
> +++ b/lib/puppet/configurer.rb
> @@ -133,7 +133,12 @@ class Puppet::Configurer
>     # This just passes any options on to the catalog,
>     # which accepts :tags and :ignoreschedules.
>     def run(options = {})
> -        prepare()
> +        begin
> +            prepare()
> +        rescue Exception => detail
> +            puts detail.backtrace if Puppet[:trace]
> +            Puppet.err "Failed to prepare catalog: %s" % detail
> +        end
>
>         if catalog = options[:catalog]
>             options.delete(:catalog)
> diff --git a/lib/puppet/indirector/facts/facter.rb b/lib/puppet/ 
> indirector/facts/facter.rb
> index 9df71fc..6c6cbc6 100644
> --- a/lib/puppet/indirector/facts/facter.rb
> +++ b/lib/puppet/indirector/facts/facter.rb
> @@ -29,7 +29,7 @@ class Puppet::Node::Facts::Facter <  
> Puppet::Indirector::Code
>                     Timeout::timeout(self.timeout) do
>                         load file
>                     end
> -                rescue => detail
> +                rescue Exception => detail
>                     Puppet.warning "Could not load fact file %s: %s"  
> % [fqfile, detail]
>                 end
>             end
> diff --git a/lib/puppet/indirector/ldap.rb b/lib/puppet/indirector/ 
> ldap.rb
> index 7485bd9..744a839 100644
> --- a/lib/puppet/indirector/ldap.rb
> +++ b/lib/puppet/indirector/ldap.rb
> @@ -1,4 +1,4 @@
> -require 'puppet/indirector/terminus'
> +requir 'puppet/indirector/terminus'
> require 'puppet/util/ldap/connection'
>
> class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
> @@ -40,7 +40,7 @@ class Puppet::Indirector::Ldap <  
> Puppet::Indirector::Terminus
>                 found = true
>                 yield entry
>             end
> -        rescue => detail
> +        rescue Exception => detail
>             if count == 0
>                 # Try reconnecting to ldap if we get an exception  
> and we haven't yet retried.
>                 count += 1
> diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
> index 29b947e..11e11c5 100644
> --- a/lib/puppet/ssl/host.rb
> +++ b/lib/puppet/ssl/host.rb
> @@ -235,12 +235,11 @@ class Puppet::SSL::Host
>
>     # Attempt to retrieve a cert, if we don't already have one.
>     def wait_for_cert(time)
> -        return if certificate
>         begin
> +            return if certificate
>             generate
> -
>             return if certificate
> -        rescue StandardError => detail
> +        rescue Exception => detail
>             Puppet.err "Could not request certificate: %s" %  
> detail.to_s
>             if time < 1
>                 puts "Exiting; failed to retrieve certificate and  
> watiforcert is disabled"
> -- 
> 1.6.4
>
>
> >


-- 
Man is ready to die for an idea, provided that idea is not quite
clear to him. --Paul Eldridge
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to