report transient errors as transient errors

The Fail method for acquire methods has a boolean parameter indicating
the transient-nature of a reported error. The problem with this is that
Fail is called very late at a point where it is no longer easily
identifiable if an error is indeed transient or not, so some calls were
and some weren't and the acquire system would later mostly ignore the
transient flag and guess by using the FailReason instead.

Introducing a tri-state enum we can pass the information about fatal or
transient errors through the callstack to generate the correct fails.

1 files changed, 72 insertions, 36 deletions

diff --git a/methods/ftp.cc b/methods/ftp.cc
index 99eebc5af..2daba8ffe 100644
--- a/methods/ftp.cc
+++ b/methods/ftp.cc
@@ -110,12 +110,12 @@ void FTPConn::Close()
 // ---------------------------------------------------------------------
 /* Connect to the server using a non-blocking connection and perform a 
    login. */
-bool FTPConn::Open(aptMethod *Owner)
+ResultState FTPConn::Open(aptMethod *Owner)
 {
    // Use the already open connection if possible.
    if (ServerFd->Fd() != -1)
-      return true;
-   
+      return ResultState::SUCCESSFUL;
+
    Close();
    
    // Determine the proxy setting
@@ -167,31 +167,40 @@ bool FTPConn::Open(aptMethod *Owner)
    /* Connect to the remote server. Since FTP is connection oriented we
       want to make sure we get a new server every time we reconnect */
    RotateDNS();
-   if (Connect(Host,Port,"ftp",21,ServerFd,TimeOut,Owner) == false)
-      return false;
+   auto result = Connect(Host, Port, "ftp", 21, ServerFd, TimeOut, Owner);
+   if (result != ResultState::SUCCESSFUL)
+      return result;
 
    // Login must be before getpeername otherwise dante won't work.
    Owner->Status(_("Logging in"));
-   bool Res = Login();
-   
+   result = Login();
+   if (result != ResultState::SUCCESSFUL)
+      return result;
+
    // Get the remote server's address
    PeerAddrLen = sizeof(PeerAddr);
    if (getpeername(ServerFd->Fd(), (sockaddr *)&PeerAddr, &PeerAddrLen) != 0)
-      return _error->Errno("getpeername",_("Unable to determine the peer name"));
-   
+   {
+      _error->Errno("getpeername", _("Unable to determine the peer name"));
+      return ResultState::TRANSIENT_ERROR;
+   }
+
    // Get the local machine's address
    ServerAddrLen = sizeof(ServerAddr);
    if (getsockname(ServerFd->Fd(), (sockaddr *)&ServerAddr, &ServerAddrLen) != 0)
-      return _error->Errno("getsockname",_("Unable to determine the local name"));
-   
-   return Res;
+   {
+      _error->Errno("getsockname", _("Unable to determine the local name"));
+      return ResultState::TRANSIENT_ERROR;
+   }
+
+   return ResultState::SUCCESSFUL;
 }
 									/*}}}*/
 // FTPConn::Login - Login to the remote server				/*{{{*/
 // ---------------------------------------------------------------------
 /* This performs both normal login and proxy login using a simples script
    stored in the config file. */
-bool FTPConn::Login()
+ResultState FTPConn::Login()
 {
    unsigned int Tag;
    string Msg;
@@ -211,22 +220,31 @@ bool FTPConn::Login()
    {
       // Read the initial response
       if (ReadResp(Tag,Msg) == false)
-	 return false;
+	 return ResultState::TRANSIENT_ERROR;
       if (Tag >= 400)
-	 return _error->Error(_("The server refused the connection and said: %s"),Msg.c_str());
-      
+      {
+	 _error->Error(_("The server refused the connection and said: %s"), Msg.c_str());
+	 return ResultState::FATAL_ERROR;
+      }
+
       // Send the user
       if (WriteMsg(Tag,Msg,"USER %s",User.c_str()) == false)
-	 return false;
+	 return ResultState::TRANSIENT_ERROR;
       if (Tag >= 400)
-	 return _error->Error(_("USER failed, server said: %s"),Msg.c_str());
-      
+      {
+	 _error->Error(_("USER failed, server said: %s"), Msg.c_str());
+	 return ResultState::FATAL_ERROR;
+      }
+
       if (Tag == 331) { // 331 User name okay, need password.
          // Send the Password
          if (WriteMsg(Tag,Msg,"PASS %s",Pass.c_str()) == false)
-            return false;
-         if (Tag >= 400)
-            return _error->Error(_("PASS failed, server said: %s"),Msg.c_str());
+	    return ResultState::TRANSIENT_ERROR;
+	 if (Tag >= 400)
+	 {
+	    _error->Error(_("PASS failed, server said: %s"), Msg.c_str());
+	    return ResultState::FATAL_ERROR;
+	 }
       }
       
       // Enter passive mode
@@ -239,15 +257,21 @@ bool FTPConn::Login()
    {      
       // Read the initial response
       if (ReadResp(Tag,Msg) == false)
-	 return false;
+	 return ResultState::TRANSIENT_ERROR;
       if (Tag >= 400)
-	 return _error->Error(_("The server refused the connection and said: %s"),Msg.c_str());
-      
+      {
+	 _error->Error(_("The server refused the connection and said: %s"), Msg.c_str());
+	 return ResultState::TRANSIENT_ERROR;
+      }
+
       // Perform proxy script execution
       Configuration::Item const *Opts = _config->Tree("Acquire::ftp::ProxyLogin");
       if (Opts == 0 || Opts->Child == 0)
-	 return _error->Error(_("A proxy server was specified but no login "
-			      "script, Acquire::ftp::ProxyLogin is empty."));
+      {
+	 _error->Error(_("A proxy server was specified but no login "
+			 "script, Acquire::ftp::ProxyLogin is empty."));
+	 return ResultState::FATAL_ERROR;
+      }
       Opts = Opts->Child;
 
       // Iterate over the entire login script
@@ -274,9 +298,12 @@ bool FTPConn::Login()
 
 	 // Send the command
 	 if (WriteMsg(Tag,Msg,"%s",Tmp.c_str()) == false)
-	    return false;
+	    return ResultState::TRANSIENT_ERROR;
 	 if (Tag >= 400)
-	    return _error->Error(_("Login script command '%s' failed, server said: %s"),Tmp.c_str(),Msg.c_str());	 
+	 {
+	    _error->Error(_("Login script command '%s' failed, server said: %s"), Tmp.c_str(), Msg.c_str());
+	    return ResultState::FATAL_ERROR;
+	 }
       }
       
       // Enter passive mode
@@ -300,11 +327,13 @@ bool FTPConn::Login()
    
    // Binary mode
    if (WriteMsg(Tag,Msg,"TYPE I") == false)
-      return false;
+      return ResultState::TRANSIENT_ERROR;
    if (Tag >= 400)
-      return _error->Error(_("TYPE failed, server said: %s"),Msg.c_str());
-   
-   return true;
+   {
+      _error->Error(_("TYPE failed, server said: %s"), Msg.c_str());
+      return ResultState::FATAL_ERROR;
+   }
+   return ResultState::SUCCESSFUL;
 }
 									/*}}}*/
 // FTPConn::ReadLine - Read a line from the server			/*{{{*/
@@ -1023,15 +1052,22 @@ bool FtpMethod::Fetch(FetchItem *Itm)
       delete Server;
       Server = new FTPConn(Get);
    }
-  
+
    // Could not connect is a transient error..
-   if (Server->Open(this) == false)
+   switch (Server->Open(this))
    {
+   case ResultState::TRANSIENT_ERROR:
       Server->Close();
       Fail(true);
       return true;
+   case ResultState::FATAL_ERROR:
+      Server->Close();
+      Fail(false);
+      return true;
+   case ResultState::SUCCESSFUL:
+      break;
    }
-   
+
    // Get the files information
    Status(_("Query"));
    unsigned long long Size;