[PATCH] Fix untidy samba-tool exception handling

[Tim Beale]

During a join, our exception handling was throwing an exception. The
resulting samba-tool output wasn't particularly user-friendly. I spotted
this problem during a failed CI autobuild run.

Attached is a fix. I also raised a bug for backporting it:
https://bugzilla.samba.org/show_bug.cgi?id=13747

CI link: https://gitlab.com/catalyst-samba/samba/pipelines/43503776

Review appreciated, thanks.

-------------- next part --------------
From c6a15f98e77242423dd0c831a076c8788456fc00 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 16 Jan 2019 15:17:38 +1300
Subject: [PATCH 1/2] join: Fix TypeError when handling exception

When we can't resolve a domain name, we were inadvertently throwing a
TypeError whilst trying to output a helpful message. E.g.

ERROR(<class 'TypeError'>): uncaught exception - 'NTSTATUSError' object
does not support indexing

Instead of indexing the object, we want to index the Exception.args so
that we just display the string portion of the exception error.

The same problem is also present for the domain trust commands.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13747

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/join.py          | 2 +-
 python/samba/netcmd/domain.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/samba/join.py b/python/samba/join.py
index cf5d1b9..28b7f0b 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -346,7 +346,7 @@ class DCJoinContext(object):
             ctx.cldap_ret = ctx.net.finddc(domain=domain, flags=nbt.NBT_SERVER_LDAP | nbt.NBT_SERVER_DS | nbt.NBT_SERVER_WRITABLE)
         except NTSTATUSError as error:
             raise Exception("Failed to find a writeable DC for domain '%s': %s" %
-                            (domain, error[1]))
+                            (domain, error.args[1]))
         except Exception:
             raise Exception("Failed to find a writeable DC for domain '%s'" % domain)
         if ctx.cldap_ret.client_site is not None and ctx.cldap_ret.client_site != "":
diff --git a/python/samba/netcmd/domain.py b/python/samba/netcmd/domain.py
index 9c5ae21..b7aedc1 100644
--- a/python/samba/netcmd/domain.py
+++ b/python/samba/netcmd/domain.py
@@ -1802,7 +1802,7 @@ class DomainTrustCommand(Command):
             remote_info = remote_net.finddc(flags=remote_flags, domain=domain, address=remote_server)
         except NTSTATUSError as error:
             raise CommandError("Failed to find a writeable DC for domain '%s': %s" %
-                               (domain, error[1]))
+                               (domain, error.args[1]))
         except Exception:
             raise CommandError("Failed to find a writeable DC for domain '%s'" % domain)
         flag_map = {
-- 
2.7.4


From 6bc17abf3cc73587da405e9435c1f768cbfa0775 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 16 Jan 2019 15:37:00 +1300
Subject: [PATCH 2/2] join: Throw CommandError instead of Exception for simple
 errors

Throwing an exception here still dumps out the Python stack trace, which
can be a little disconcerting for users.

In this case, the stack trace isn't going to really help at all (the
problem is pretty obvious), and it obscures the useful message
explaining what went wrong.

Throw a CommandError instead, which samba-tool will catch and display
more nicely.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13747

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/join.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/python/samba/join.py b/python/samba/join.py
index 28b7f0b..da8dcb0 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -50,6 +50,7 @@ import os
 import tempfile
 from samba.compat import text_type
 from samba.compat import get_string
+from samba.netcmd import CommandError
 
 
 class DCJoinException(Exception):
@@ -345,10 +346,10 @@ class DCJoinContext(object):
         try:
             ctx.cldap_ret = ctx.net.finddc(domain=domain, flags=nbt.NBT_SERVER_LDAP | nbt.NBT_SERVER_DS | nbt.NBT_SERVER_WRITABLE)
         except NTSTATUSError as error:
-            raise Exception("Failed to find a writeable DC for domain '%s': %s" %
-                            (domain, error.args[1]))
+            raise CommandError("Failed to find a writeable DC for domain '%s': %s" %
+                               (domain, error.args[1]))
         except Exception:
-            raise Exception("Failed to find a writeable DC for domain '%s'" % domain)
+            raise CommandError("Failed to find a writeable DC for domain '%s'" % domain)
         if ctx.cldap_ret.client_site is not None and ctx.cldap_ret.client_site != "":
             ctx.site = ctx.cldap_ret.client_site
         return ctx.cldap_ret.pdc_dns_name
-- 
2.7.4