bug#62044: 30.0.50; ERC 5.5: Auto-reconnect is broken

["J.P." <jp@neverwas.me>]

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

"J.P." <jp@neverwas.me> writes:

> In your initial report, underlying connectivity is present in some
> form because ERC reaches the "Logging in as" stage and attempts to
> send an application payload,

Actually, this is nonsense (forgive me). I seem to have forgotten that
ERC prints this message regardless of whether a connection attempt
succeeds.

> So to keep things sane, we should probably treat auto-reconnecting
> across connectivity gaps as a wishlist item and auto-reconnecting atop
> healthy connections as an existing, previously unreported bug. As for
> the new feature, I've attached a POC patch that you can try if you're
> willing (usage is self-explanatory, but feel free to modify or iterate
> as needed).

I've attached a less sloppy version that probably still fails in some
common cases, but at least it reuses the existing session connector.

> As for the other issue, I think we're going to need some genuine
> session logs to really get anywhere. That is, we'll likely need you to
> enable logging and tracing during a real session with your bouncer
> over a hopefully not-too-prolonged period to capture an actual
> reconnect sequence failing. But hold off on that for another round
> unless you're feeling adventurous.

Actually, I'm not sure we'll be needing these logs after all. Let's
maybe think on it a bit more (if that's acceptable). Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v1-v2.diff --]
[-- Type: text/x-patch, Size: 3886 bytes --]

From 01a2aa830b73028aecf1f7dae7dadba7467a3144 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Wed, 8 Mar 2023 18:04:16 -0800
Subject: [PATCH 0/1] *** NOT A PATCH ***

*** BLURB HERE ***

F. Jason Park (1):
  Add conditional erc-server-reconnect-function

 lisp/erc/erc-backend.el | 56 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

Interdiff:
diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index d1738e4f92d..d289df98bab 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -749,6 +749,9 @@ erc--server-reconnect-timeout
 (defun erc-server-delayed-check-reconnect (buffer)
   "Wait for internet connectivity before trying to reconnect.
 BUFFER is the server buffer for the current connection."
+  ;; This may appear to hang for a good while at various places
+  ;; because it calls wait_reading_process_output a bunch.  It does at
+  ;; least sometimes print "Waiting for socket from ..." or similar.
   (with-current-buffer buffer
     (setq erc--server-reconnect-timeout
           (min 300 (* (or erc--server-reconnect-timeout
@@ -763,26 +766,38 @@ erc-server-delayed-check-reconnect
                  (erc-display-message nil 'error buffer "Nobody home...")
                  (erc-schedule-reconnect buffer 0))))))
       (condition-case _
-          (make-network-process
-           :name "*erc-connectivity-check"
-           :host erc-session-server
-           :service erc-session-port
-           :nowait t
-           :filter
-           (lambda (proc _)
-             (delete-process proc)
-             (with-current-buffer buffer
-               (setq erc--server-reconnect-timeout nil))
-             (run-at-time nil nil #'erc-server-delayed-reconnect buffer))
-           :sentinel
-           (lambda (cproc event)
-             (with-current-buffer buffer
-               (pcase event
-                 ("open\n"
-                  (run-at-time nil nil #'send-string
-                               cproc "PING *connect-check*\r\n"))
-                 ("connection broken by remote peer\n"
-                  (funcall reschedule cproc))))))
+          (let ((proc (funcall erc-session-connector
+                               "*erc-connectivity-check" nil
+                               erc-session-server erc-session-port
+                               :nowait t))
+                tls-check)
+            (when (and (not (eq erc-session-connector
+                                #'erc-open-network-stream))
+                       (process-contact proc :tls-parameters))
+              (setq tls-check
+                    (run-at-time
+                     1 1 (lambda (proc)
+                           (unless (eq 'connect (process-status proc))
+                             (cancel-timer tls-check))
+                           (when (eq 'failed (process-status proc))
+                             (funcall reschedule proc)))
+                     proc)))
+            (set-process-filter
+             proc (lambda (proc _)
+                    (delete-process proc)
+                    (with-current-buffer buffer
+                      (setq erc--server-reconnect-timeout nil))
+                    (run-at-time nil nil #'erc-server-delayed-reconnect
+                                 buffer)))
+            (set-process-sentinel
+             proc (lambda (cproc event)
+                    (with-current-buffer buffer
+                      (pcase event
+                        ("open\n"
+                         (run-at-time nil nil #'send-string
+                                      cproc "PING *connect-check*\r\n"))
+                        ("connection broken by remote peer\n"
+                         (funcall reschedule cproc)))))))
         (file-error (funcall reschedule nil))))))
 
 (defun erc-server-filter-function (process string)
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Add-conditional-erc-server-reconnect-function.patch --]
[-- Type: text/x-patch, Size: 3738 bytes --]

From 01a2aa830b73028aecf1f7dae7dadba7467a3144 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Wed, 8 Mar 2023 06:14:36 -0800
Subject: [PATCH 1/1] Add conditional erc-server-reconnect-function

* lisp/erc/erc-backend.el (erc--server-reconnect-timer,
erc-server-delayed-check-reconnect): Add possible alternate value for
option `erc-server-reconnect-function' that only attempts to reconnect
after hearing back from the server.  Also add helper variable.
---
 lisp/erc/erc-backend.el | 56 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 567443f5329..d289df98bab 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -744,6 +744,62 @@ erc-server-delayed-reconnect
     (with-current-buffer buffer
       (erc-server-reconnect))))
 
+(defvar-local erc--server-reconnect-timeout nil)
+
+(defun erc-server-delayed-check-reconnect (buffer)
+  "Wait for internet connectivity before trying to reconnect.
+BUFFER is the server buffer for the current connection."
+  ;; This may appear to hang for a good while at various places
+  ;; because it calls wait_reading_process_output a bunch.  It does at
+  ;; least sometimes print "Waiting for socket from ..." or similar.
+  (with-current-buffer buffer
+    (setq erc--server-reconnect-timeout
+          (min 300 (* (or erc--server-reconnect-timeout
+                          erc-server-reconnect-timeout)
+                      2)))
+    (let ((reschedule
+           (lambda (proc)
+             (let ((erc-server-reconnect-timeout
+                    erc--server-reconnect-timeout))
+               (with-current-buffer buffer
+                 (delete-process proc)
+                 (erc-display-message nil 'error buffer "Nobody home...")
+                 (erc-schedule-reconnect buffer 0))))))
+      (condition-case _
+          (let ((proc (funcall erc-session-connector
+                               "*erc-connectivity-check" nil
+                               erc-session-server erc-session-port
+                               :nowait t))
+                tls-check)
+            (when (and (not (eq erc-session-connector
+                                #'erc-open-network-stream))
+                       (process-contact proc :tls-parameters))
+              (setq tls-check
+                    (run-at-time
+                     1 1 (lambda (proc)
+                           (unless (eq 'connect (process-status proc))
+                             (cancel-timer tls-check))
+                           (when (eq 'failed (process-status proc))
+                             (funcall reschedule proc)))
+                     proc)))
+            (set-process-filter
+             proc (lambda (proc _)
+                    (delete-process proc)
+                    (with-current-buffer buffer
+                      (setq erc--server-reconnect-timeout nil))
+                    (run-at-time nil nil #'erc-server-delayed-reconnect
+                                 buffer)))
+            (set-process-sentinel
+             proc (lambda (cproc event)
+                    (with-current-buffer buffer
+                      (pcase event
+                        ("open\n"
+                         (run-at-time nil nil #'send-string
+                                      cproc "PING *connect-check*\r\n"))
+                        ("connection broken by remote peer\n"
+                         (funcall reschedule cproc)))))))
+        (file-error (funcall reschedule nil))))))
+
 (defun erc-server-filter-function (process string)
   "The process filter for the ERC server."
   (with-current-buffer (process-buffer process)
-- 
2.39.2