stoneCC / serf

Automatically exported from code.google.com/p/serf
Apache License 2.0
0 stars 0 forks source link

'svn co http://127.0.0.1:12345' should return a better error than APR_EGENERAL for connection failed #108

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. svn co http://127.0.0.1:12345

What is the expected output? What do you see instead?
Something like

$ svn info http://127.0.0.1:12345/
svn: E730061: Unable to connect to a repository at URL 'http://127.0.0.1:12345'
svn: E730061: Error running context: No connection could be made because the 
target machine actively refused it.
(Which maps to WSAECONNREFUSED)

What version of the product are you using? On what operating system?
1.2.0 on Windows 8

The following patch fixes this problem on Window and FreeBSD and probably most 
other *nix platforms.
(It would be nice if apr provided a common function for this pattern as it is 
now already duplicated in several apr source files)

Index: outgoing.c
===================================================================
--- outgoing.c  (revision 1950)
+++ outgoing.c  (working copy)
@@ -16,6 +16,7 @@
 #include <apr_pools.h>
 #include <apr_poll.h>
 #include <apr_version.h>
+#include <apr_portable.h>

 #include "serf.h"
 #include "serf_bucket_util.h"
@@ -1137,6 +1138,20 @@
         if (conn->completed_requests && !conn->probable_keepalive_limit) {
             return reset_connection(conn, 1);
         }
+#ifdef SO_ERROR
+        {
+          apr_os_sock_t *h;
+          if (!apr_os_sock_get(&h, conn->skt))
+            {
+              int rc;
+              int rclen = sizeof(rc);
+              if (getsockopt(h, SOL_SOCKET, SO_ERROR, (char*) &rc, &rclen)) {
+                return apr_get_netos_error();
+              }
+                return APR_FROM_OS_ERROR(rc);
+            }
+        }
+#endif
         return APR_EGENERAL;
     }
     if ((events & APR_POLLOUT) != 0) {

Original issue reported on code.google.com by b...@qqmail.nl on 24 Jun 2013 at 7:10

GoogleCodeExporter commented 9 years ago
The apr_get_netos_error() returns the error why getsockopt failed. It might be 
better to remove that case and handle getsockopt errors and the general case 
with a new serf errorcode.

Original comment by b...@qqmail.nl on 24 Jun 2013 at 7:11

GoogleCodeExporter commented 9 years ago
I couldn't reproduce this issue on Mac OS X or Linux, only on Windows. I've 
updated the patch a bit to make it build without warnings (apr_os_sock_get 
takes a *apr_os_sock_t, not **apr_os_sock_t), tested it in Windows and it works 
as expected.

I've left the APR_EGENERAL as is for now, can't introduce new error codes in 
1.2.x anyway. If I don't forget I'll cleanup all APR_EGENERAL return locations 
before 1.3.

Thanks Bert.

Lieven

Original comment by lieven.govaerts@gmail.com on 24 Jun 2013 at 8:35