Use a better method of setting the internal error that automatically

adjusts for trailing newlines and exception detritus, saving duplicate
code.  Standardize the documentation of the error() method and document
using this in child classes of the generic ACL and Object classes.

Disable printing of errors during connect in Wallet::Server since we're
going to throw our own exception.

6 files changed, 128 insertions, 118 deletions

diff --git a/perl/Wallet/ACL.pm b/perl/Wallet/ACL.pm
index b6b6ee5..a0417f8 100644
--- a/perl/Wallet/ACL.pm
+++ b/perl/Wallet/ACL.pm
@@ -104,9 +104,15 @@ sub create {
 # Utility functions
 ##############################################################################
 
-# Returns the current error message of the object, if any.
+# Set or return the error stashed in the object.
 sub error {
-    my ($self) = @_;
+    my ($self, @error) = @_;
+    if (@error) {
+        my $error = join ('', @error);
+        chomp $error;
+        1 while ($error =~ s/ at \S+ line \d+\.?\z//);
+        $self->{error} = $error;
+    }
     return $self->{error};
 }
 
@@ -148,7 +154,7 @@ sub log_acl {
 sub rename {
     my ($self, $name) = @_;
     if ($name =~ /^\d+\z/) {
-        $self->{error} = "ACL name may not be all numbers";
+        $self->error ("ACL name may not be all numbers");
         return undef;
     }
     eval {
@@ -157,9 +163,7 @@ sub rename {
         $self->{dbh}->commit;
     };
     if ($@) {
-        $self->{error} = "cannot rename ACL $self->{id} to $name: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot rename ACL $self->{id} to $name: $@");
         $self->{dbh}->rollback;
         return undef;
     }
@@ -183,9 +187,7 @@ sub destroy {
         $self->{dbh}->commit;
     };
     if ($@) {
-        $self->{error} = "cannot destroy ACL $self->{id}: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot destroy ACL $self->{id}: $@");
         $self->{dbh}->rollback;
         return undef;
     }
@@ -201,7 +203,7 @@ sub add {
     my ($self, $scheme, $identifier, $user, $host, $time) = @_;
     $time ||= time;
     unless ($MAPPING{$scheme}) {
-        $self->{error} = "unknown ACL scheme $scheme";
+        $self->error ("unknown ACL scheme $scheme");
         return undef;
     }
     eval {
@@ -212,9 +214,7 @@ sub add {
         $self->{dbh}->commit;
     };
     if ($@) {
-        $self->{error} = "cannot add $scheme:$identifier to $self->{id}: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot add $scheme:$identifier to $self->{id}: $@");
         $self->{dbh}->rollback;
         return undef;
     }
@@ -243,9 +243,7 @@ sub remove {
     };
     if ($@) {
         my $entry = "$scheme:$identifier";
-        $self->{error} = "cannot remove $entry from $self->{id}: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot remove $entry from $self->{id}: $@");
         $self->{dbh}->rollback;
         return undef;
     }
@@ -273,9 +271,7 @@ sub list {
         }
     };
     if ($@) {
-        $self->{error} = "cannot retrieve ACL $self->{id}: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot retrieve ACL $self->{id}: $@");
         return (undef);
     } else {
         return @entries;
@@ -294,7 +290,7 @@ sub list {
 sub check {
     my ($self, $principal) = @_;
     unless ($principal) {
-        $self->{error} = 'no principal specified';
+        $self->error ('no principal specified');
         return undef;
     }
     my @entries = $self->list;
@@ -448,7 +444,9 @@ DATETIME isn't given, the current time is used.
 
 =item error()
 
-Returns the error text of the last error.
+Returns the error of the last failing operation or undef if no operations
+have failed.  Callers should call this function to get the error message
+after an undef return from any other instance method.
 
 =item id()
 
diff --git a/perl/Wallet/ACL/Base.pm b/perl/Wallet/ACL/Base.pm
index dfc6b60..a03086d 100644
--- a/perl/Wallet/ACL/Base.pm
+++ b/perl/Wallet/ACL/Base.pm
@@ -41,9 +41,15 @@ sub check {
     return 0;
 }
 
-# Return the error stashed in the object.
+# Set or return the error stashed in the object.
 sub error {
-    my ($self) = @_;
+    my ($self, @error) = @_;
+    if (@error) {
+        my $error = join ('', @error);
+        chomp $error;
+        1 while ($error =~ s/ at \S+ line \d+\.?\z//);
+        $self->{error} = $error;
+    }
     return $self->{error};
 }
 
@@ -87,11 +93,18 @@ and blesses an object.
 This method should always be overridden by child classes.  The default
 implementation just declines all access.
 
-=item error()
+=item error([ERROR ...])
+
+Returns the error of the last failing operation or undef if no operations
+have failed.  Callers should call this function to get the error message
+after an undef return from any other instance method.
 
-Returns whatever is stored in the error key of the object hash.  Child
-classes should store error messages in that key when returning undef from
-check().
+For the convenience of child classes, this method can also be called with
+one or more error strings.  If so, those strings are concatenated together,
+trailing newlines are removed, any text of the form S<C< at \S+ line
+\d+\.?>> at the end of the message is stripped off, and the result is stored
+as the error.  Only child classes should call this method with an error
+string.
 
 =back
 
diff --git a/perl/Wallet/ACL/Krb5.pm b/perl/Wallet/ACL/Krb5.pm
index ffe1fc5..eab0c28 100644
--- a/perl/Wallet/ACL/Krb5.pm
+++ b/perl/Wallet/ACL/Krb5.pm
@@ -34,11 +34,11 @@ $VERSION = '0.01';
 sub check {
     my ($self, $principal, $acl) = @_;
     unless ($principal) {
-        $self->{error} = 'no principal specified';
+        $self->error ('no principal specified');
         return undef;
     }
     unless ($acl) {
-        $self->{error} = 'malformed krb5 ACL';
+        $self->error ('malformed krb5 ACL');
         return undef;
     }
     return ($principal eq $acl) ? 1 : 0;
@@ -80,7 +80,7 @@ principal if and only if the principal exactly matches the ACL.
 
 =item new()
 
-Creates a new ACL verifier.  The database handle is not used.
+Creates a new ACL verifier.  For this verifier, there is no setup work.
 
 =item check(PRINCIPAL, ACL)
 
diff --git a/perl/Wallet/Object/Base.pm b/perl/Wallet/Object/Base.pm
index c3514e3..941e34a 100644
--- a/perl/Wallet/Object/Base.pm
+++ b/perl/Wallet/Object/Base.pm
@@ -88,9 +88,15 @@ sub create {
 # Utility functions
 ##############################################################################
 
-# Returns the current error message of the object, if any.
+# Set or return the error stashed in the object.
 sub error {
-    my ($self) = @_;
+    my ($self, @error) = @_;
+    if (@error) {
+        my $error = join ('', @error);
+        chomp $error;
+        1 while ($error =~ s/ at \S+ line \d+\.?\z//);
+        $self->{error} = $error;
+    }
     return $self->{error};
 }
 
@@ -115,7 +121,7 @@ sub name {
 sub log_action {
     my ($self, $action, $user, $host, $time) = @_;
     unless ($action =~ /^(get|store)\z/) {
-        $self->{error} = "invalid history action $action";
+        $self->error ("invalid history action $action");
         return undef;
     }
 
@@ -143,9 +149,7 @@ sub log_action {
     };
     if ($@) {
         my $id = $self->{type} . ':' . $self->{name};
-        $self->{error} = "cannot update history for $id: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot update history for $id: $@");
         $self->{dbh}->rollback;
         return undef;
     }
@@ -188,7 +192,7 @@ sub log_set {
 sub _set_internal {
     my ($self, $attr, $value, $user, $host, $time) = @_;
     if ($attr !~ /^[a-z_]+\z/) {
-        $self->{error} = "invalid attribute $attr";
+        $self->error ("invalid attribute $attr");
         return;
     }
     $time ||= time;
@@ -206,9 +210,7 @@ sub _set_internal {
     };
     if ($@) {
         my $id = $self->{type} . ':' . $self->{name};
-        $self->{error} = "cannot set $attr on $id: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*//;
+        $self->error ("cannot set $attr on $id: $@");
         $self->{dbh}->rollback;
         return;
     }
@@ -219,7 +221,7 @@ sub _set_internal {
 sub _get_internal {
     my ($self, $attr) = @_;
     if ($attr !~ /^[a-z_]+\z/) {
-        $self->{error} = "invalid attribute $attr";
+        $self->error ("invalid attribute $attr");
         return;
     }
     $attr = 'ob_' . $attr;
@@ -235,9 +237,7 @@ sub _get_internal {
 sub acl {
     my ($self, $type, $id, $user, $host, $time) = @_;
     if ($type !~ /^(get|store|show|destroy|flags)\z/) {
-        $self->{error} = "invalid ACL type $type";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("invalid ACL type $type");
         return;
     }
     my $attr = "acl_$type";
@@ -245,9 +245,7 @@ sub acl {
         my $acl;
         eval { $acl = Wallet::ACL->new ($id, $self->{dbh}) };
         if ($@) {
-            $self->{error} = $@;
-            chomp $self->{error};
-            $self->{error} =~ s/ at .*$//;
+            $self->error ($@);
             return undef;
         }
         return $self->_set_internal ($attr, $acl->id, $user, $host, $time);
@@ -265,7 +263,7 @@ sub expires {
     my ($self, $expires, $user, $host, $time) = @_;
     if ($expires) {
         if ($expires !~ /^\d+\z/ || $expires == 0) {
-            $self->{error} = "malformed expiration time $expires";
+            $self->error ("malformed expiration time $expires");
             return;
         }
         return $self->_set_internal ('expires', $expires, $user, $host, $time);
@@ -284,9 +282,7 @@ sub owner {
         my $acl;
         eval { $acl = Wallet::ACL->new ($owner, $self->{dbh}) };
         if ($@) {
-            $self->{error} = $@;
-            chomp $self->{error};
-            $self->{error} =~ s/ at .*$//;
+            $self->error ($@);
             return undef;
         }
         return $self->_set_internal ('owner', $acl->id, $user, $host, $time);
@@ -309,7 +305,7 @@ sub get { die "Do not instantiate Wallet::Object::Base directly\n"; }
 sub store {
     my ($self, $data, $user, $host, $time) = @_;
     my $id = $self->{type} . ':' . $self->{name};
-    $self->{error} = "cannot store $id: object type is immutable";
+    $self->error ("cannot store $id: object type is immutable");
     return;
 }
 
@@ -346,9 +342,7 @@ sub show {
         @data = $self->{dbh}->selectrow_array ($sql, undef, $type, $name);
     };
     if ($@) {
-        $self->{error} = "cannot retrieve data for ${type}:${name}: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot retrieve data for ${type}:${name}: $@");
         return undef;
     }
     my $output = '';
@@ -384,9 +378,7 @@ sub destroy {
         $self->{dbh}->commit;
     };
     if ($@) {
-        $self->{error} = "cannot destroy ${type}:${name}: $@";
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ("cannot destroy ${type}:${name}: $@");
         $self->{dbh}->rollback;
         return undef;
     }
@@ -487,6 +479,19 @@ true on success and false on failure.  The arguments are used for logging
 and history and should indicate the user and host from which the change is
 made and the time of the change.
 
+=item error([ERROR ...])
+
+Returns the error of the last failing operation or undef if no operations
+have failed.  Callers should call this function to get the error message
+after an undef return from any other instance method.
+
+For the convenience of child classes, this method can also be called with
+one or more error strings.  If so, those strings are concatenated together,
+trailing newlines are removed, any text of the form S<C< at \S+ line
+\d+\.?>> at the end of the message is stripped off, and the result is stored
+as the error.  Only child classes should call this method with an error
+string.
+
 =item error()
 
 Returns the error message from the last failing operation or undef if no
diff --git a/perl/Wallet/Object/Keytab.pm b/perl/Wallet/Object/Keytab.pm
index 313a439..38e0938 100644
--- a/perl/Wallet/Object/Keytab.pm
+++ b/perl/Wallet/Object/Keytab.pm
@@ -123,7 +123,7 @@ sub _kadmin_addprinc {
 sub _kadmin_ktadd {
     my ($self, $principal, $file) = @_;
     unless ($self->_valid_principal ($principal)) {
-        $self->{error} = "invalid principal name: $principal";
+        $self->error ("invalid principal name: $principal");
         return undef;
     }
     if ($Wallet::Config::KEYTAB_REALM) {
@@ -131,11 +131,10 @@ sub _kadmin_ktadd {
     }
     my $output = eval { $self->_kadmin ("ktadd -q -k $file $principal") };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
+        $self->error ($@);
         return undef;
     } elsif ($output =~ /^(?:kadmin|ktadd): (.*)/m) {
-        $self->{error} = "error creating keytab for $principal: $1";
+        $self->error ("error creating keytab for $principal: $1");
         return undef;
     }
     return 1;
@@ -147,13 +146,12 @@ sub _kadmin_ktadd {
 sub _kadmin_delprinc {
     my ($self, $principal) = @_;
     unless ($self->_valid_principal ($principal)) {
-        $self->{error} = "invalid principal name: $principal";
+        $self->error ("invalid principal name: $principal");
         return undef;
     }
     my $exists = eval { $self->_kadmin_exists ($principal) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
+        $self->error ($@);
         return undef;
     } elsif (not $exists) {
         return 1;
@@ -163,11 +161,10 @@ sub _kadmin_delprinc {
     }
     my $output = eval { $self->_kadmin ("delprinc -force $principal") };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
+        $self->error ($@);
         return undef;
     } elsif ($output =~ /^delete_principal: (.*)/m) {
-        $self->{error} = "error deleting $principal: $1";
+        $self->error ("error deleting $principal: $1");
         return undef;
     }
     return 1;
@@ -200,7 +197,7 @@ sub get {
     my ($self, $user, $host, $time) = @_;
     $time ||= time;
     unless (defined ($Wallet::Config::KEYTAB_TMP)) {
-        $self->{error} = 'KEYTAB_TMP configuration variable not set';
+        $self->error ('KEYTAB_TMP configuration variable not set');
         return undef;
     }
     my $file = $Wallet::Config::KEYTAB_TMP . "/keytab.$$";
@@ -208,7 +205,7 @@ sub get {
     local *KEYTAB;
     unless (open (KEYTAB, '<', $file)) {
         my $princ = $self->{name};
-        $self->{error} = "error opening keytab for principal $princ: $!";
+        $self->error ("error opening keytab for principal $princ: $!");
         return undef;
     }
     local $/;
@@ -216,7 +213,7 @@ sub get {
     my $data = <KEYTAB>;
     if ($!) {
         my $princ = $self->{name};
-        $self->{error} = "error reading keytab for principal $princ: $!";
+        $self->error ("error reading keytab for principal $princ: $!");
         return undef;
     }
     close KEYTAB;
diff --git a/perl/Wallet/Server.pm b/perl/Wallet/Server.pm
index 6bf4251..8cbc139 100644
--- a/perl/Wallet/Server.pm
+++ b/perl/Wallet/Server.pm
@@ -55,8 +55,9 @@ sub _open_db {
         $dsn .= ";host=$Wallet::Config::DB_HOST" if $Wallet::Config::DB_HOST;
         $dsn .= ";port=$Wallet::Config::DB_PORT" if $Wallet::Config::DB_PORT;
     }
-    my $dbh = DBI->connect ($dsn, $Wallet::Config::DB_USER,
-                            $Wallet::Config::DB_PASSWORD);
+    my $user = $Wallet::Config::DB_USER;
+    my $password = $Wallet::Config::DB_PASSWORD;
+    my $dbh = DBI->connect ($dsn, $user, $password, { PrintError => 0 });
     if (not defined $dbh) {
         die "cannot connect to database: $DBI::errstr\n";
     }
@@ -110,9 +111,15 @@ sub dbh {
     return $self->{dbh};
 }
 
-# Returns the error from the previous failed operation.
+# Set or return the error stashed in the object.
 sub error {
-    my ($self) = @_;
+    my ($self, @error) = @_;
+    if (@error) {
+        my $error = join ('', @error);
+        chomp $error;
+        1 while ($error =~ s/ at \S+ line \d+\.?\z//);
+        $self->{error} = $error;
+    }
     return $self->{error};
 }
 
@@ -134,7 +141,7 @@ sub DESTROY {
 sub create {
     my ($self, $type, $name) = @_;
     unless ($MAPPING{$type}) {
-        $self->{error} = "unknown object type $type";
+        $self->error ("unknown object type $type");
         return undef;
     }
     my $class = $MAPPING{$type};
@@ -142,14 +149,12 @@ sub create {
     my $user = $self->{user};
     my $host = $self->{host};
     unless ($self->{admin}->check ($user)) {
-        $self->{error} = "$user not authorized to create ${type}:${name}";
+        $self->error ("$user not authorized to create ${type}:${name}");
         return undef;
     }
     my $object = eval { $class->create ($type, $name, $dbh, $user, $host) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     } else {
         return 1;
@@ -161,15 +166,13 @@ sub create {
 sub retrieve {
     my ($self, $type, $name) = @_;
     unless ($MAPPING{$type}) {
-        $self->{error} = "unknown object type $type";
+        $self->error ("unknown object type $type");
         return undef;
     }
     my $class = $MAPPING{$type};
     my $object = eval { $class->new ($type, $name, $self->{dbh}) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     } else {
         return $object;
@@ -185,7 +188,7 @@ sub object_error {
     if ($action !~ /^(create|get|store|show|destroy)\z/) {
         $action = "set $action for";
     }
-    $self->{error} = "$self->{user} not authorized to $action $id";
+    $self->error ("$self->{user} not authorized to $action $id");
 }
 
 # Given an object and an action, checks if the current user has access to
@@ -196,7 +199,7 @@ sub object_error {
 sub acl_check {
     my ($self, $object, $action) = @_;
     unless ($action =~ /^(get|store|show|destroy|flags)\z/) {
-        $self->{error} = "unknown action $action";
+        $self->error ("unknown action $action");
         return undef;
     }
     if ($action ne 'get' and $action ne 'store') {
@@ -212,16 +215,14 @@ sub acl_check {
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     }
     my $status = $acl->check ($self->{user});
     if ($status == 1) {
         return 1;
     } elsif (not defined $status) {
-        $self->{error} = $acl->error;
+        $self->error ($acl->error);
         return undef;
     } else {
         $self->object_error ($object, $action);
@@ -245,7 +246,9 @@ sub acl {
     } else {
         $result = $object->acl ($acl);
     }
-    $self->{error} = $object->error unless defined $result;
+    if (not defined ($result) and $object->error) {
+        $self->error ($object->error);
+    }
     return $result;
 }
 
@@ -265,7 +268,9 @@ sub expires {
     } else {
         $result = $object->expires;
     }
-    $self->{error} = $object->error unless defined $result;
+    if (not defined ($result) and $object->error) {
+        $self->error ($object->error);
+    }
     return $result;
 }
 
@@ -285,7 +290,9 @@ sub owner {
     } else {
         $result = $object->owner;
     }
-    $self->{error} = $object->error unless defined $result;
+    if (not defined ($result) and $object->error) {
+        $self->error ($object->error);
+    }
     return $result;
 }
 
@@ -298,7 +305,7 @@ sub get {
     return undef unless defined $object;
     return undef unless $self->acl_check ($object, 'get');
     my $result = $object->get ($self->{user}, $self->{host});
-    $self->{error} = $object->error unless defined $result;
+    $self->error ($object->error) unless defined $result;
     return $result;
 }
 
@@ -315,7 +322,7 @@ sub store {
         return undef;
     }
     my $result = $object->store ($data, $self->{user}, $self->{host});
-    $self->{error} = $object->error unless defined $result;
+    $self->error ($object->error) unless defined $result;
     return $result;
 }
 
@@ -328,7 +335,7 @@ sub show {
     return undef unless defined $object;
     return undef unless $self->acl_check ($object, 'show');
     my $result = $object->show;
-    $self->{error} = $object->error unless defined $result;
+    $self->error ($object->error) unless defined $result;
     return $result;
 }
 
@@ -340,7 +347,7 @@ sub destroy {
     return undef unless defined $object;
     return undef unless $self->acl_check ($object, 'destroy');
     my $result = $object->destroy ($self->{user}, $self->{host});
-    $self->{error} = $object->error unless defined $result;
+    $self->error ($object->error) unless defined $result;
     return $result;
 }
 
@@ -353,7 +360,7 @@ sub destroy {
 sub acl_create {
     my ($self, $name) = @_;
     unless ($self->{admin}->check ($self->{user})) {
-        $self->{error} = "$self->{user} not authorized to create ACL";
+        $self->error ("$self->{user} not authorized to create ACL");
         return undef;
     }
     my $dbh = $self->{dbh};
@@ -361,9 +368,7 @@ sub acl_create {
     my $host = $self->{host};
     my $acl = eval { Wallet::ACL->create ($name, $dbh, $user, $host) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     } else {
         return 1;
@@ -380,7 +385,7 @@ sub acl_error {
     } elsif ($action eq 'remove') {
         $action = 'remove from';
     }
-    $self->{error} = "$self->{user} not authorized to $action ACL $acl";
+    $self->error ("$self->{user} not authorized to $action ACL $acl");
 }
 
 # Change the human-readable name of an ACL or return undef and set the
@@ -393,13 +398,11 @@ sub acl_rename {
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     }
     unless ($acl->rename ($name)) {
-        $self->{error} = $acl->error;
+        $self->error ($acl->error);
         return undef;
     }
     return 1;
@@ -415,13 +418,11 @@ sub acl_destroy {
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     }
     unless ($acl->destroy ($self->{user}, $self->{host})) {
-        $self->{error} = $acl->error;
+        $self->error ($acl->error);
         return undef;
     }
     return 1;
@@ -437,13 +438,11 @@ sub acl_add {
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     }
     unless ($acl->add ($scheme, $identifier, $self->{user}, $self->{host})) {
-        $self->{error} = $acl->error;
+        $self->error ($acl->error);
         return undef;
     }
     return 1;
@@ -459,15 +458,13 @@ sub acl_remove {
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
-        $self->{error} = $@;
-        chomp $self->{error};
-        $self->{error} =~ s/ at .*$//;
+        $self->error ($@);
         return undef;
     }
     my $user = $self->{user};
     my $host = $self->{host};
     unless ($acl->remove ($scheme, $identifier, $user, $host)) {
-        $self->{error} = $acl->error;
+        $self->error ($acl->error);
         return undef;
     }
     return 1;