Exception handling advise needed for samba-tool

[Jelmer Vernooij]

On Tue, Apr 09, 2013 at 12:01:21AM -0700, Matthieu Patou wrote:
> On 04/08/2013 04:18 PM, Andrew Bartlett wrote:
> >I'm trying to make the exceptions from samba-tool domain classicupgrade
> >less intimidating, so I tried this:
> >
> >diff --git a/python/samba/netcmd/domain.py
> >b/python/samba/netcmd/domain.py
> >index 4ba305c..c47b7be 100644
> >--- a/python/samba/netcmd/domain.py
> >+++ b/python/samba/netcmd/domain.py
> >@@ -1313,9 +1313,12 @@ class cmd_domain_classicupgrade(Command):
> >          s3conf.load(smbconf)
> >          samba3 = Samba3(smbconf, s3conf)
> >-        logger.info("Provisioning")
> >-        upgrade_from_samba3(samba3, logger, targetdir,
> >session_info=system_session(),
> >-                            useeadb=eadb, dns_backend=dns_backend,
> >use_ntvfs=use_ntvfs)
> >+        logger.info("Preparing to Upgrade")
> >+        try:
> >+            upgrade_from_samba3(samba3, logger, targetdir,
> >session_info=system_session(),
> >+                                useeadb=eadb, dns_backend=dns_backend,
> >use_ntvfs=use_ntvfs)
> >+        except ProvisioningError, e:
> >+            raise CommandError("Upgrade failed: ", e)
> >But I still get:
> >
> >ERROR(<class 'samba.provision.ProvisioningError'>): Upgrade failed:  -
> >ProvisioningError: Could not open '/tmp/secrets.tdb', the Samba3 secrets
> >database: [Errno 2] No such file or directory.  Perhaps you specified
> >the incorrect smb.conf, --testparm or --dbdir option?
> >   File "bin/python/samba/netcmd/domain.py", line 1319, in run
> >     useeadb=eadb, dns_backend=dns_backend, use_ntvfs=use_ntvfs)
> >   File "bin/python/samba/upgrade.py", line 575, in upgrade_from_samba3
> >     raise ProvisioningError("Could not open '%s', the Samba3 secrets
> >database: %s.  Perhaps you specified the incorrect smb.conf, --testparm
> >or --dbdir option?" % (samba3.privatedir_path("secrets.tdb"), str(e)))
> >
> >Can you remind me of the correct syntax to turn that into just:
> >
> >Upgrade failed: Could not open '/tmp/secrets.tdb', the Samba3 secrets
> >database: [Errno 2] No such file or directory.  Perhaps you specified
> >the incorrect smb.conf, --testparm or --dbdir option?
> I think that as long as you raise an exception you will get the call
> trace, try to see if you can just print the exception or just get
> the error message.
> 
> You can have a look at samba_upgradeprovision around line 1160 I did
> that there.
The infrastructure is meant to deal with exceptions and display them properly.
They shouldn't be caught and written to standard output manually.

Having a single place where exceptions are handled means we can avoid
reinventing the wheel everywhere and e.g. have environment variables to
force the dumping of tracebacks or having Python bring up a debugger whenever
an exception is raised. It also makes whitebox testing of the code easier
since you don't have to parse the output but you can just catch the
exception and introspect it.

How samba-tool handles various exceptions should really be documented better.
Sorry. The ubershort summary:

 * CommandError exceptions are expected, should only be raised by the
   command code (not library code), should have a user-comprehensible
   description. They're meant for general user-visible errors. When they're
   printed there is no traceback printed.
 * Any other internal errors should be caught by the command code
   and re-raise a CommandError. You can attach an internal exception to a
   CommandError for debugging purposes.
 * Any internal errors that are not explicitly caught are reported with a
   traceback. They're the Python equivalent of a segmentation error.

(it's gotten more complicated over time, and we special case more exceptions
 than just CommandError now. I don't have the source code with me to
 look at the moment)

Cheers,

Jelmer