master f5ebe47ba77 1/2: Reuse process in erc-server-delayed-check-reconnect

[F. Jason Park]

branch: master
commit f5ebe47ba7723919f09adf8f28d896cfa8757842
Author: F. Jason Park <jp@neverwas.me>
Commit: F. Jason Park <jp@neverwas.me>

    Reuse process in erc-server-delayed-check-reconnect
    
    * doc/misc/erc.texi (Integrations): Set `erc-server-reconnect-function'
    to `erc-server-delayed-check-reconnect' in SOCKS example, and add
    definition for `erc-open-socks-tls-stream'.  Mention possible
    inaccuracies related to error detection with certain reconnect
    strategies.
    * lisp/erc/erc-backend.el (erc-server--reconnect-opened): New function.
    (erc-server-delayed-check-reconnect): Attempt to reuse process if server
    sends a complete PONG, and attempt to accommodate connectors that set
    :nowait to nil.
    (erc--server-delayed-check-connectors): Remove variable.
    (erc-server-prefer-check-reconnect): Inline what was the internal
    variable `erc--server-delayed-check-connectors' because it's no longer
    used in unit tests.  Add `erc-open-socks-tls-stream' to the set of
    connector functions thought to be compatible with the "check" reconnect
    strategy.
    * test/lisp/erc/erc-scenarios-base-auto-recon.el
    (erc-scenarios-base-auto-recon-no-proto): Adapt to expect "reuse" behavior.
    * test/lisp/erc/resources/base/reconnect/ping-pong.eld: Delete unused file.
    * test/lisp/erc/resources/base/reconnect/unexpected-disconnect.eld:
    Capture PING cookie to send back to client.
    * test/lisp/erc/resources/erc-d/resources/proxy-solo.eld: Delete unused
    file.  (Bug#62044)
---
 doc/misc/erc.texi                                  | 14 +++++
 lisp/erc/erc-backend.el                            | 62 +++++++++++++++-------
 test/lisp/erc/erc-scenarios-base-auto-recon.el     |  3 +-
 .../erc/resources/base/reconnect/ping-pong.eld     |  6 ---
 .../base/reconnect/unexpected-disconnect.eld       |  3 +-
 .../erc/resources/erc-d/resources/proxy-solo.eld   |  9 ----
 6 files changed, 62 insertions(+), 35 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index b5838b3b0ce..0045691a230 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -1667,6 +1667,8 @@ shown in the following example:
         (socks-server '("tor" "localhost" 9050 5)))
     (apply #'erc-open-socks-tls-stream args)))
 
+(setopt erc-server-reconnect-function #'erc-server-delayed-check-reconnect)
+
 (let* ((erc-modules (cons 'sasl erc-modules))
        (erc-sasl-mechanism 'external)
        (erc-server-connect-function #'my-erc-open-socks-tls-stream))
@@ -1687,6 +1689,18 @@ with ERC exclusively, you can just set those options and 
variables
 globally and bind @code{erc-server-connect-function} to
 @code{erc-open-socks-tls-stream} instead.
 
+@defun erc-open-socks-tls-stream
+The default TLS @dfn{connector} for @acronym{SOCKS} connections.
+Compatible with option @code{erc-server-connect-function}.  Be aware
+that if used in conjunction with certain values of
+@code{erc-server-reconnect-function} (such as
+@code{erc-server-prefer-check-reconnect}, currently the default
+@dfn{reconnector}, or @code{erc-server-delayed-check-reconnect}, the
+more specialized workhorse it defers to), this function may cause ERC to
+misreport proxy-related failures as routine lapses in internet
+connectivity.
+@end defun
+
 @node auth-source
 @subsection auth-source
 @cindex auth-source
diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index ab26a3715f1..098cf6d7d71 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -830,6 +830,13 @@ Make sure you are in an ERC buffer when running this."
     (with-current-buffer buffer
       (erc-server-reconnect))))
 
+(defun erc-server--reconnect-opened (buffer process)
+  "Reconnect session for server BUFFER using open PROCESS."
+  (when (buffer-live-p buffer)
+    (with-current-buffer buffer
+      (let ((erc-session-connector (lambda (&rest _) process)))
+        (erc-server-reconnect)))))
+
 (defvar-local erc--server-reconnect-timeout nil)
 (defvar-local erc--server-reconnect-timeout-check 10)
 (defvar-local erc--server-reconnect-timeout-scale-function
@@ -845,7 +852,13 @@ Make sure you are in an ERC buffer when running this."
 
 (defun erc-server-delayed-check-reconnect (buffer)
   "Wait for internet connectivity before trying to reconnect.
-Expect BUFFER to be the server buffer for the current connection."
+Use server BUFFER's cached session info to reestablish the logical
+connection at the IRC protocol level.  Do this by probing for a
+successful response to a PING before commencing with \"connection
+registration\".  Do not distinguish between configuration problems and
+the absence of service.  For example, expect users of proxy-based
+connectors, like `erc-open-socks-tls-stream', to ensure their setup
+works before choosing this function as their reconnector."
   (when (buffer-live-p buffer)
     (with-current-buffer buffer
       (setq erc--server-reconnect-timeout
@@ -857,7 +870,8 @@ Expect BUFFER to be the server buffer for the current 
connection."
                              (with-current-buffer buffer
                                (let ((erc-server-reconnect-timeout
                                       erc--server-reconnect-timeout))
-                                 (delete-process proc)
+                                 (when proc ; conn refused w/o :nowait
+                                   (delete-process proc))
                                  (erc-display-message nil 'error buffer
                                                       "Nobody home...")
                                  (erc-schedule-reconnect buffer 0))))))
@@ -867,7 +881,7 @@ Expect BUFFER to be the server buffer for the current 
connection."
              (conchk (lambda (proc)
                        (let ((status (process-status proc))
                              (xprdp (time-less-p conchk-exp (current-time))))
-                         (when (or (not (eq 'connect status)) xprdp)
+                         (when (or xprdp (not (eq 'connect status)))
                            (cancel-timer conchk-timer))
                          (when (buffer-live-p buffer)
                            (cond (xprdp (erc-display-message
@@ -880,38 +894,50 @@ Expect BUFFER to be the server buffer for the current 
connection."
              (sentinel (lambda (proc event)
                          (pcase event
                            ("open\n"
-                            (run-at-time nil nil #'process-send-string proc
-                                         (format "PING %d\r\n"
-                                                 (time-convert nil 'integer))))
+                            (let ((cookie (time-convert nil 'integer)))
+                              (process-put proc 'erc--reconnect-cookie cookie)
+                              (run-at-time nil nil #'process-send-string proc
+                                           (format "PING %d\r\n" cookie))))
                            ((or "connection broken by remote peer\n"
                                 (rx bot "failed"))
                             (run-at-time nil nil reschedule proc)))))
-             (filter (lambda (proc _)
-                       (delete-process proc)
+             (filter (lambda (proc string)
                        (with-current-buffer buffer
                          (setq erc--server-reconnect-timeout nil))
-                       (run-at-time nil nil #'erc-server-delayed-reconnect
-                                    buffer))))
+                       (if-let* ; reuse proc if string has complete message
+                           ((cookie (process-get proc 'erc--reconnect-cookie))
+                            ((string-suffix-p (format "PONG %d\r\n" cookie)
+                                              string))) ; leading ":<source> "
+                           (progn
+                             (erc-log-irc-protocol string nil)
+                             (set-process-sentinel proc #'ignore)
+                             (set-process-filter proc nil)
+                             (run-at-time nil nil
+                                          #'erc-server--reconnect-opened
+                                          buffer proc))
+                         (delete-process proc)
+                         (run-at-time nil nil #'erc-server-delayed-reconnect
+                                      buffer)))))
         (condition-case _
             (let ((proc (funcall erc-session-connector
                                  "*erc-connectivity-check*" nil
-                                 erc-session-server erc-session-port
-                                 :nowait t)))
+                                 erc-session-server erc-session-port)))
               (setq conchk-timer (run-at-time 1 1 conchk proc))
               (set-process-filter proc filter)
-              (set-process-sentinel proc sentinel))
+              (set-process-sentinel proc sentinel)
+              (when (eq (process-status proc) 'open) ; :nowait is nil
+                (funcall sentinel proc "open\n")))
+          ;; E.g., "make client process failed" "Connection refused".
           (file-error (funcall reschedule nil)))))))
 
-(defvar erc--server-delayed-check-connectors
-  '(erc-open-tls-stream erc-open-network-stream)
-  "Functions compatible with `erc-server-delayed-check-reconnect'.")
-
 (defun erc-server-prefer-check-reconnect (buffer)
   "Defer to another reconnector based on BUFFER's `erc-session-connector'.
 Prefer `erc-server-delayed-check-reconnect' if the connector is known to
 be \"check-aware\".  Otherwise, use `erc-server-delayed-reconnect'."
   (if (memq (buffer-local-value 'erc-session-connector buffer)
-            erc--server-delayed-check-connectors)
+            '(erc-open-tls-stream
+              erc-open-network-stream
+              erc-open-socks-tls-stream))
       (erc-server-delayed-check-reconnect buffer)
     (erc-server-delayed-reconnect buffer)))
 
diff --git a/test/lisp/erc/erc-scenarios-base-auto-recon.el 
b/test/lisp/erc/erc-scenarios-base-auto-recon.el
index d6a114147b3..adde30e6d47 100644
--- a/test/lisp/erc/erc-scenarios-base-auto-recon.el
+++ b/test/lisp/erc/erc-scenarios-base-auto-recon.el
@@ -100,6 +100,8 @@
       ((erc-server-flood-penalty 0.1)
        (erc-scenarios-common-dialog "base/reconnect")
        (erc-d-auto-pong nil)
+       (erc-d-tmpl-vars
+        `((cookie . ,(lambda (a) (funcall a :set (funcall a :match 1))))))
        (dumb-server (erc-d-run "localhost" t 'unexpected-disconnect))
        (port (process-contact dumb-server :service))
        (erc--server-reconnect-timeout-scale-function (lambda (_) 1))
@@ -128,7 +130,6 @@
     (ert-info ("Service restored")
       (setq dumb-server (erc-d-run "localhost" port
                                    'just-ping
-                                   'ping-pong
                                    'unexpected-disconnect))
       (with-current-buffer "FooNet"
         (funcall expect 30 "server is in debug mode")))
diff --git a/test/lisp/erc/resources/base/reconnect/ping-pong.eld 
b/test/lisp/erc/resources/base/reconnect/ping-pong.eld
deleted file mode 100644
index b3d36cf6cec..00000000000
--- a/test/lisp/erc/resources/base/reconnect/ping-pong.eld
+++ /dev/null
@@ -1,6 +0,0 @@
-;; -*- mode: lisp-data; -*-
-((ping 10 "PING ")
- (0 "PONG fake"))
-
-((eof 10 EOF))
-((drop 0 DROP))
diff --git a/test/lisp/erc/resources/base/reconnect/unexpected-disconnect.eld 
b/test/lisp/erc/resources/base/reconnect/unexpected-disconnect.eld
index 80903f94155..281239dfdaa 100644
--- a/test/lisp/erc/resources/base/reconnect/unexpected-disconnect.eld
+++ b/test/lisp/erc/resources/base/reconnect/unexpected-disconnect.eld
@@ -1,7 +1,8 @@
 ;; -*- mode: lisp-data; -*-
 
 ((~eof 60 EOF))
-((~ping 60 "PING"))
+((~ping 60 "PING " (group (+ (in "0-9"))))
+ (0 "PONG " cookie))
 
 ((nick 10 "NICK tester"))
 ((user 10 "USER user 0 * :tester")
diff --git a/test/lisp/erc/resources/erc-d/resources/proxy-solo.eld 
b/test/lisp/erc/resources/erc-d/resources/proxy-solo.eld
deleted file mode 100644
index af216c80edc..00000000000
--- a/test/lisp/erc/resources/erc-d/resources/proxy-solo.eld
+++ /dev/null
@@ -1,9 +0,0 @@
-;;; -*- mode: lisp-data -*-
-
-((pass 10.0 "PASS " (? ?:) "changeme"))
-((nick 0.2 "NICK tester"))
-
-((user 0.2 "USER user 0 * :" (group (+ alpha)) eos)
- (0 ":*status!znc@znc.in NOTICE " nick " :You have no networks configured."
-    " Use /znc AddNetwork <network> to add one.")
- (0 ":irc.znc.in 001 " nick " :Welcome " nick "!"))