nandi/frq · 91b0e1e

nandi/frqpublic⑂ Fork 0

⑂ 91b0e1e

Dialling is a blocking call too

`⚠ Interrupted system call` on pressing Connect, still. The retries went
on the reads and the writes and both HTTPS paths, and not on the one
call that happens first: `connect`, which is a DNS lookup, a TCP connect
and a TLS handshake, every one of them interruptible and all of them
before a byte has been read.

The whole socket setup is inside the retry rather than the connect
alone, because a socket that failed part way through cannot be dialled
again — each attempt starts with a new one and closes the last.

Two things came out of looking at this properly.

A failed refresh no longer throws the saved sign-in away unless the
broker is the thing saying no. It used to drop the token on any
exception at all, so a dropped wifi or a stray signal cost the reader
their session and sent them back to a browser. Only an `OauthError` —
the broker refusing the token — means the token is bad; everything else
is the network, and the token is still good.

And errors out of the socket thread now say what could not be done.
"Interrupted system call" on its own names no call, which is why this
took two rounds of guessing: it now reads "Could not reach
irc.freeq.at:6697 — …", and the sign-in path says the broker.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandi committed 2026-09-19T17:50:10-07:00 Browse files

91b0e1e parent: 4083860

modified nim/src/frq/conn.nim +24 -7 Viewed

@@ -75,12 +75,24 @@ proc readerBody(cfg: ConnConfig) {.thread.} =
75	75	try:
76	76	trace("conn", "dialling " & cfg.host & ":" & $cfg.port &
77	77	(if cfg.tls: " over TLS" else: " plain"))
78		- var sock = newSocket(buffered = true)
79		- if cfg.tls:
80		- # CVerifyPeer: this carries a nick and, once SASL is wired, a token.
81		- let ctx = newContext(verifyMode = CVerifyPeer)
82		- ctx.wrapSocket(sock)
83		- sock.connect(cfg.host, Port(cfg.port))
	78	+ # Dialling is the first blocking call a connection makes and the one
	79	+ # most likely to be cut short: a DNS lookup, a TCP connect and a TLS
	80	+ # handshake, all of them interruptible, all of them before anything has
	81	+ # been read. `⚠ Interrupted system call` on pressing Connect was this.
	82	+ #
	83	+ # The whole setup is inside the retry rather than the connect alone: a
	84	+ # socket that failed part way through cannot be dialled again, so each
	85	+ # attempt starts with a new one and closes the last.
	86	+ var sock: Socket
	87	+ retrying 3:
	88	+ if not sock.isNil:
	89	+ try: sock.close() except CatchableError: discard
	90	+ sock = newSocket(buffered = true)
	91	+ if cfg.tls:
	92	+ # CVerifyPeer: this carries a nick and a token.
	93	+ let ctx = newContext(verifyMode = CVerifyPeer)
	94	+ ctx.wrapSocket(sock)
	95	+ sock.connect(cfg.host, Port(cfg.port))
84	96	shared = sock
85	97	createThread(writer, writerBody, 0)
86	98	events.send("open")
@@ -105,8 +117,13 @@ proc readerBody(cfg: ConnConfig) {.thread.} =
105	117	inbound.send(line)
106	118
107	119	except CatchableError as e:
	120	+ # Named for what failed. Every error out of this thread used to arrive
	121	+ # on the connect screen as a bare OS message — "Interrupted system
	122	+ # call" on its own says nothing about which call, and it took two
	123	+ # rounds of guessing to find out.
108	124	trace("conn", "!! " & e.msg)
109		- events.send("error: " & e.msg)
	125	+ events.send("error: Could not reach " & cfg.host & ":" & $cfg.port &
	126	+ " — " & e.msg)
110	127	finally:
111	128	running = false
112	129	# Unblock the writer, which is sitting in `outbound.recv()`.

      try:
        trace("conn", "dialling " & cfg.host & ":" & $cfg.port &
                      (if cfg.tls: " over TLS" else: " plain"))
--      var sock = newSocket(buffered = true)
++      # Dialling is the first blocking call a connection makes and the one
--      if cfg.tls:
++      # most likely to be cut short: a DNS lookup, a TCP connect and a TLS
--        # CVerifyPeer: this carries a nick and, once SASL is wired, a token.
++      # handshake, all of them interruptible, all of them before anything has
--        let ctx = newContext(verifyMode = CVerifyPeer)
++      # been read. `⚠ Interrupted system call` on pressing Connect was this.
--        ctx.wrapSocket(sock)
++      #
--      sock.connect(cfg.host, Port(cfg.port))
++      # The whole setup is inside the retry rather than the connect alone: a
++      # socket that failed part way through cannot be dialled again, so each
++      # attempt starts with a new one and closes the last.
++      var sock: Socket
++      retrying 3:
++        if not sock.isNil:
++          try: sock.close() except CatchableError: discard
++        sock = newSocket(buffered = true)
++        if cfg.tls:
++          # CVerifyPeer: this carries a nick and a token.
++          let ctx = newContext(verifyMode = CVerifyPeer)
++          ctx.wrapSocket(sock)
++        sock.connect(cfg.host, Port(cfg.port))
        shared = sock
        createThread(writer, writerBody, 0)
        events.send("open")
          inbound.send(line)
      except CatchableError as e:
++      # Named for what failed. Every error out of this thread used to arrive
++      # on the connect screen as a bare OS message — "Interrupted system
++      # call" on its own says nothing about which call, and it took two
++      # rounds of guessing to find out.
        trace("conn", "!! " & e.msg)
--      events.send("error: " & e.msg)
++      events.send("error: Could not reach " & cfg.host & ":" & $cfg.port &
++                  " — " & e.msg)
      finally:
        running = false
        # Unblock the writer, which is sitting in `outbound.recv()`.

modified nim/src/frq/reducer.nim +15 -5 Viewed

@@ -228,16 +228,26 @@ proc signIn(): bool =
228	228	app.status = "Refreshing your sign-in…"
229	229	adoptTokens(oa.refreshSession(oa.defaultBroker, app.brokerToken))
230	230	true
231		- except CatchableError as e:
232		- # A token the broker no longer honours is worse than none: every
233		- # Connect would spend a round trip failing the same way. Dropped, and
234		- # the next press opens the browser.
235		- trace("oauth", "refresh failed: " & e.msg)
	231	+ except oa.OauthError as e:
	232	+ # The broker itself saying no. A token it no longer honours is worse
	233	+ # than none — every Connect would spend a round trip failing the same
	234	+ # way — so it goes, and the next press opens the browser.
	235	+ trace("oauth", "refresh refused: " & e.msg)
236	236	app.brokerToken = ""
237	237	clearSession()
238	238	setError(e.msg)
239	239	app.connecting = false
240	240	false
	241	+ except CatchableError as e:
	242	+ # Anything else is the network, not the answer: a name that did not
	243	+ # resolve, a connection that did not open, a syscall a signal cut
	244	+ # short. The token is still good and is kept — throwing it away here
	245	+ # meant a dropped wifi or a stray SIGPROF cost the reader their saved
	246	+ # sign-in and sent them back to a browser.
	247	+ trace("oauth", "refresh failed: " & e.msg)
	248	+ setError("Could not reach the broker — " & e.msg)
	249	+ app.connecting = false
	250	+ false
241	251
242	252	proc openSocket() =
243	253	## The connection itself, with whoever we are already settled.

        app.status = "Refreshing your sign-in…"
        adoptTokens(oa.refreshSession(oa.defaultBroker, app.brokerToken))
        true
--    except CatchableError as e:
++    except oa.OauthError as e:
--      # A token the broker no longer honours is worse than none: every
++      # The broker itself saying no. A token it no longer honours is worse
--      # Connect would spend a round trip failing the same way. Dropped, and
++      # than none — every Connect would spend a round trip failing the same
--      # the next press opens the browser.
++      # way — so it goes, and the next press opens the browser.
--      trace("oauth", "refresh failed: " & e.msg)
++      trace("oauth", "refresh refused: " & e.msg)
        app.brokerToken = ""
        clearSession()
        setError(e.msg)
        app.connecting = false
        false
++    except CatchableError as e:
++      # Anything else is the network, not the answer: a name that did not
++      # resolve, a connection that did not open, a syscall a signal cut
++      # short. The token is still good and is kept — throwing it away here
++      # meant a dropped wifi or a stray SIGPROF cost the reader their saved
++      # sign-in and sent them back to a browser.
++      trace("oauth", "refresh failed: " & e.msg)
++      setError("Could not reach the broker — " & e.msg)
++      app.connecting = false
++      false
  proc openSocket() =
    ## The connection itself, with whoever we are already settled.