nandi/frq · abb8e36

nandi/frqpublic⑂ Fork 0

⑂ abb8e36

A signal is not a failure

`⚠ Interrupted system call` on the connect screen, with a disconnect
behind it. EINTR is what a blocking syscall returns when a signal
arrives while it is waiting: nothing has gone wrong and the call simply
has to be made again. This client treated it as the connection breaking.

It shows up in the app and never here, which is the whole problem. A Nim
test binary has no signals to speak of; the app is a Flutter process,
and the Dart VM's profiler sends SIGPROF to every thread in it — ours
included — hundreds of times a second. Whether that interrupts a syscall
or resumes it is decided by an SA_RESTART flag on a handler this code
neither installs nor can see.

So `frq/eintr` tells the two apart, and the blocking calls retry: the
socket read, the socket write, both HTTPS paths, and the loopback
listener's accept. The distinction is kept narrow — only EINTR is
retried, and only a bounded number of times, because something raising
it for ever should fail where it can be seen rather than spin.

`interrupted` looks at `errorCode` where there is one and at the message
otherwise: `httpclient` catches an OSError in places and re-raises its
text, which keeps the words and drops the code.

I could not reproduce this from a test — a handled SIGALRM on the main
thread does not interrupt a read on the socket thread, and reaching that
thread from outside `conn` is not something the module allows. What the
tests check is the judgement rather than the syscall: which errors are
retried, which are raised at once, and that a retry gives up.

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

nandi committed 2026-09-19T10:31:52-07:00 Browse files

abb8e36 parent: 60bbd35

modified nim/src/frq/atproto.nim +11 -5 Viewed

@@ -10,7 +10,7 @@
10	10	## JSON and neither could be depended on; one language has one JSON.
11	11
12	12	import std/[base64, httpclient, json, net, strutils]
13		-import frq/trace
	13	+import frq/[trace, eintr]
14	14
15	15	const
16	16	directoryHost* = "public.api.bsky.app"
@@ -52,7 +52,11 @@ proc getJson(url, whatFor: string): JsonNode =
52	52	trace("atproto", "GET " & url)
53	53	let c = newClient()
54	54	try:
55		- parseJson(c.getContent(url))
	55	+ var body: string
	56	+ # Retried where a signal cut the round trip short; see `frq/eintr`.
	57	+ retrying 3:
	58	+ body = c.getContent(url)
	59	+ parseJson(body)
56	60	except JsonParsingError:
57	61	raise newException(AtprotoError, whatFor & " — the server's answer was not JSON.")
58	62	except CatchableError as e:
@@ -129,9 +133,11 @@ proc createSession*(handle, password: string): Session =
129	133	try:
130	134	c.headers = newHttpHeaders({"Content-Type": "application/json"})
131	135	let payload = $(%*{"identifier": handle.strip(), "password": password})
132		- let res = c.request("https://" & hostOf(pds) &
133		- "/xrpc/com.atproto.server.createSession",
134		- httpMethod = HttpPost, body = payload)
	136	+ var res: Response
	137	+ retrying 3:
	138	+ res = c.request("https://" & hostOf(pds) &
	139	+ "/xrpc/com.atproto.server.createSession",
	140	+ httpMethod = HttpPost, body = payload)
135	141	# Read the body whatever the status: the PDS puts the reason in it, and a
136	142	# wrong app password is a 401 whose message is the useful part.
137	143	let body = try: parseJson(res.body)

  ## JSON and neither could be depended on; one language has one JSON.
  import std/[base64, httpclient, json, net, strutils]
--import frq/trace
++import frq/[trace, eintr]
  const
    directoryHost* = "public.api.bsky.app"
    trace("atproto", "GET " & url)
    let c = newClient()
    try:
--    parseJson(c.getContent(url))
++    var body: string
++    # Retried where a signal cut the round trip short; see `frq/eintr`.
++    retrying 3:
++      body = c.getContent(url)
++    parseJson(body)
    except JsonParsingError:
      raise newException(AtprotoError, whatFor & " — the server's answer was not JSON.")
    except CatchableError as e:
    try:
      c.headers = newHttpHeaders({"Content-Type": "application/json"})
      let payload = $(%*{"identifier": handle.strip(), "password": password})
--    let res = c.request("https://" & hostOf(pds) &
++    var res: Response
--                        "/xrpc/com.atproto.server.createSession",
++    retrying 3:
--                        httpMethod = HttpPost, body = payload)
++      res = c.request("https://" & hostOf(pds) &
++                      "/xrpc/com.atproto.server.createSession",
++                      httpMethod = HttpPost, body = payload)
      # Read the body whatever the status: the PDS puts the reason in it, and a
      # wrong app password is a 401 whose message is the useful part.
      let body = try: parseJson(res.body)

modified nim/src/frq/conn.nim +8 -2 Viewed

@@ -17,7 +17,7 @@
17	17
18	18	import std/net
19	19	import std/posix as p ## for `shutdown(2)`; see `close`
20		-import frq/[trace]
	20	+import frq/[trace, eintr]
21	21
22	22	type
23	23	ConnConfig* = object
@@ -64,7 +64,8 @@ proc writerBody(unused: int) {.thread.} =
64	64	if shared.isNil: continue
65	65	try:
66	66	trace("conn.out", line)
67		- shared.send(line & "\c\L")
	67	+ retrying 5:
	68	+ shared.send(line & "\c\L")
68	69	except CatchableError as e:
69	70	events.send("error: " & e.msg)
70	71	break
@@ -90,6 +91,11 @@ proc readerBody(cfg: ConnConfig) {.thread.} =
90	91	try:
91	92	line = sock.recvLine()
92	93	except CatchableError as e:
	94	+ # A signal arriving while we waited is not the server going away.
	95	+ # The Dart VM profiles every thread in the process, this one
	96	+ # included, so a read that is interrupted and then reported as a
	97	+ # broken connection is a disconnect several times a minute.
	98	+ if interrupted(e) and running: continue
93	99	if running: events.send("error: " & e.msg)
94	100	break
95	101	if line.len == 0:

  import std/net
  import std/posix as p   ## for `shutdown(2)`; see `close`
--import frq/[trace]
++import frq/[trace, eintr]
  type
    ConnConfig* = object
        if shared.isNil: continue
        try:
          trace("conn.out", line)
--        shared.send(line & "\c\L")
++        retrying 5:
++          shared.send(line & "\c\L")
        except CatchableError as e:
          events.send("error: " & e.msg)
          break
          try:
            line = sock.recvLine()
          except CatchableError as e:
++          # A signal arriving while we waited is not the server going away.
++          # The Dart VM profiles every thread in the process, this one
++          # included, so a read that is interrupted and then reported as a
++          # broken connection is a disconnect several times a minute.
++          if interrupted(e) and running: continue
            if running: events.send("error: " & e.msg)
            break
          if line.len == 0:

added nim/src/frq/eintr.nim +47 -0 Viewed

new file mode 100644
@@ -0,0 +1,47 @@
	1	+## A signal is not a failure.
	2	+##
	3	+## `EINTR` is what a blocking syscall returns when a signal arrives while it
	4	+## is waiting: nothing has gone wrong, the call simply has to be made again.
	5	+## Nim turns it into an `OSError` reading "Interrupted system call", and
	6	+## everything here used to treat that as the connection breaking — so a
	7	+## socket read, an HTTPS round trip or a sign-in would fail with `⚠
	8	+## Interrupted system call` on the screen and a disconnect behind it.
	9	+##
	10	+## Nothing in the Nim test suite ever sees one, which is the whole problem:
	11	+## a test binary has no signals to speak of. The app is a Flutter process,
	12	+## and the Dart VM's profiler sends SIGPROF to every thread in it, ours
	13	+## included, hundreds of times a second. Whether a given signal interrupts a
	14	+## syscall or resumes it is decided by an `SA_RESTART` flag on a handler this
	15	+## code does not install and cannot see.
	16	+##
	17	+## So: retry, and keep the distinction narrow. Only EINTR is retried; every
	18	+## other error is the failure it says it is.
	19	+
	20	+import std/strutils
	21	+import std/posix as p
	22	+
	23	+func interrupted*(e: ref CatchableError): bool =
	24	+ ## Whether this error is only a signal having arrived.
	25	+ ##
	26	+ ## By `errorCode` where there is one, and by message otherwise: a library
	27	+ ## that catches an OSError and re-raises its text — `httpclient` does this
	28	+ ## in places — loses the code but keeps the words.
	29	+ if e of OSError:
	30	+ ((ref OSError)(e)).errorCode.int32 == p.EINTR.int32
	31	+ else:
	32	+ "Interrupted system call" in e.msg
	33	+
	34	+template retrying*(attempts: int, body: untyped): untyped =
	35	+ ## Run `body`, again if a signal cut it short.
	36	+ ##
	37	+ ## A bounded count rather than a loop: if something really is raising EINTR
	38	+ ## for ever, failing is better than spinning where nobody can see it.
	39	+ var tries = 0
	40	+ while true:
	41	+ tries.inc
	42	+ try:
	43	+ body
	44	+ break
	45	+ except CatchableError as e:
	46	+ if tries < attempts and interrupted(e): continue
	47	+ raise

++## A signal is not a failure.
++##
++## `EINTR` is what a blocking syscall returns when a signal arrives while it
++## is waiting: nothing has gone wrong, the call simply has to be made again.
++## Nim turns it into an `OSError` reading "Interrupted system call", and
++## everything here used to treat that as the connection breaking — so a
++## socket read, an HTTPS round trip or a sign-in would fail with `⚠
++## Interrupted system call` on the screen and a disconnect behind it.
++##
++## Nothing in the Nim test suite ever sees one, which is the whole problem:
++## a test binary has no signals to speak of. The app is a Flutter process,
++## and the Dart VM's profiler sends SIGPROF to every thread in it, ours
++## included, hundreds of times a second. Whether a given signal interrupts a
++## syscall or resumes it is decided by an `SA_RESTART` flag on a handler this
++## code does not install and cannot see.
++##
++## So: retry, and keep the distinction narrow. Only EINTR is retried; every
++## other error is the failure it says it is.
++
++import std/strutils
++import std/posix as p
++
++func interrupted*(e: ref CatchableError): bool =
++  ## Whether this error is only a signal having arrived.
++  ##
++  ## By `errorCode` where there is one, and by message otherwise: a library
++  ## that catches an OSError and re-raises its text — `httpclient` does this
++  ## in places — loses the code but keeps the words.
++  if e of OSError:
++    ((ref OSError)(e)).errorCode.int32 == p.EINTR.int32
++  else:
++    "Interrupted system call" in e.msg
++
++template retrying*(attempts: int, body: untyped): untyped =
++  ## Run `body`, again if a signal cut it short.
++  ##
++  ## A bounded count rather than a loop: if something really is raising EINTR
++  ## for ever, failing is better than spinning where nobody can see it.
++  var tries = 0
++  while true:
++    tries.inc
++    try:
++      body
++      break
++    except CatchableError as e:
++      if tries < attempts and interrupted(e): continue
++      raise

modified nim/src/frq/oauth.nim +11 -4 Viewed

@@ -25,7 +25,7 @@
25	25
26	26	import std/[base64, httpclient, json, nativesockets, net, osproc,
27	27	strutils, times]
28		-import frq/trace
	28	+import frq/[trace, eintr]
29	29
30	30	const
31	31	defaultBroker* = "https://auth.freeq.at"
@@ -119,9 +119,11 @@ proc refreshSession*(broker, brokerToken: string): Tokens =
119	119	sslContext = newContext(verifyMode = CVerifyPeer))
120	120	try:
121	121	c.headers = newHttpHeaders({"Content-Type": "application/json"})
122		- let res = c.request("https://" & brokerHost(broker) & "/session",
123		- httpMethod = HttpPost,
124		- body = $(%*{"broker_token": brokerToken}))
	122	+ var res: Response
	123	+ retrying 3:
	124	+ res = c.request("https://" & brokerHost(broker) & "/session",
	125	+ httpMethod = HttpPost,
	126	+ body = $(%*{"broker_token": brokerToken}))
125	127	# The body whatever the status: an expired token is a 401 whose message
126	128	# is the part worth showing.
127	129	let j = try: parseJson(res.body)
@@ -247,6 +249,8 @@ proc workerBody(req: LoginReq) {.thread.} =
247	249
248	250	var client: Socket
249	251	try:
	252	+ # A signal during accept or the read that follows is not the browser
	253	+ # failing to arrive; the wait goes on. See `frq/eintr`.
250	254	server.accept(client)
251	255	let (head, body) = readRequest(client)
252	256	if head.startsWith("POST"):
@@ -269,6 +273,9 @@ proc workerBody(req: LoginReq) {.thread.} =
269	273	else:
270	274	client.send(httpResponse("200 OK", "text/html; charset=utf-8",
271	275	captureHtml()))
	276	+ except CatchableError as e:
	277	+ if not interrupted(e): raise
	278	+ trace("oauth", "a signal cut a request short; still waiting")
272	279	finally:
273	280	try: client.close() except CatchableError: discard
274	281

  import std/[base64, httpclient, json, nativesockets, net, osproc,
              strutils, times]
--import frq/trace
++import frq/[trace, eintr]
  const
    defaultBroker* = "https://auth.freeq.at"
                          sslContext = newContext(verifyMode = CVerifyPeer))
    try:
      c.headers = newHttpHeaders({"Content-Type": "application/json"})
--    let res = c.request("https://" & brokerHost(broker) & "/session",
++    var res: Response
--                        httpMethod = HttpPost,
++    retrying 3:
--                        body = $(%*{"broker_token": brokerToken}))
++      res = c.request("https://" & brokerHost(broker) & "/session",
++                      httpMethod = HttpPost,
++                      body = $(%*{"broker_token": brokerToken}))
      # The body whatever the status: an expired token is a 401 whose message
      # is the part worth showing.
      let j = try: parseJson(res.body)
        var client: Socket
        try:
++        # A signal during accept or the read that follows is not the browser
++        # failing to arrive; the wait goes on. See `frq/eintr`.
          server.accept(client)
          let (head, body) = readRequest(client)
          if head.startsWith("POST"):
          else:
            client.send(httpResponse("200 OK", "text/html; charset=utf-8",
                                     captureHtml()))
++      except CatchableError as e:
++        if not interrupted(e): raise
++        trace("oauth", "a signal cut a request short; still waiting")
        finally:
          try: client.close() except CatchableError: discard

added nim/tests/teintr.nim +58 -0 Viewed

new file mode 100644
@@ -0,0 +1,58 @@
	1	+## Telling a signal apart from a failure.
	2	+##
	3	+## Nothing in this suite raises a real EINTR — a test binary has no signals
	4	+## worth the name, which is exactly why the app hit this and the tests never
	5	+## did. So the errors here are built by hand, and what is checked is the
	6	+## judgement: which ones are retried, which are not, and that a retry gives up
	7	+## rather than spinning.
	8	+
	9	+import std/[os, unittest]
	10	+import std/posix as p
	11	+import frq/eintr
	12	+
	13	+proc osError(code: cint, msg: string): ref OSError =
	14	+ result = newException(OSError, msg)
	15	+ result.errorCode = code.int32
	16	+
	17	+suite "interrupted":
	18	+ test "an OSError carrying EINTR is only a signal":
	19	+ check interrupted(osError(p.EINTR, "Interrupted system call"))
	20	+ test "any other OSError is a real failure":
	21	+ check not interrupted(osError(p.ECONNRESET, "Connection reset by peer"))
	22	+ check not interrupted(osError(p.EPIPE, "Broken pipe"))
	23	+ test "and so is an error that is not an OSError at all":
	24	+ check not interrupted(newException(ValueError, "nonsense"))
	25	+ test "but the words count where the code was lost":
	26	+ # `httpclient` catches an OSError in places and re-raises its text, which
	27	+ # keeps the message and drops the code.
	28	+ check interrupted(newException(IOError, "Interrupted system call"))
	29	+
	30	+suite "retrying":
	31	+ test "a body that works runs once":
	32	+ var runs = 0
	33	+ retrying 3:
	34	+ runs.inc
	35	+ check runs == 1
	36	+
	37	+ test "one cut short by a signal is run again":
	38	+ var runs = 0
	39	+ retrying 3:
	40	+ runs.inc
	41	+ if runs < 3: raise osError(p.EINTR, "Interrupted system call")
	42	+ check runs == 3
	43	+
	44	+ test "a real failure is raised at once, not retried":
	45	+ var runs = 0
	46	+ expect OSError:
	47	+ retrying 5:
	48	+ runs.inc
	49	+ raise osError(p.ECONNREFUSED, "Connection refused")
	50	+ check runs == 1
	51	+
	52	+ test "and a signal that never stops gives up rather than spinning":
	53	+ var runs = 0
	54	+ expect OSError:
	55	+ retrying 4:
	56	+ runs.inc
	57	+ raise osError(p.EINTR, "Interrupted system call")
	58	+ check runs == 4

++## Telling a signal apart from a failure.
++##
++## Nothing in this suite raises a real EINTR — a test binary has no signals
++## worth the name, which is exactly why the app hit this and the tests never
++## did. So the errors here are built by hand, and what is checked is the
++## judgement: which ones are retried, which are not, and that a retry gives up
++## rather than spinning.
++
++import std/[os, unittest]
++import std/posix as p
++import frq/eintr
++
++proc osError(code: cint, msg: string): ref OSError =
++  result = newException(OSError, msg)
++  result.errorCode = code.int32
++
++suite "interrupted":
++  test "an OSError carrying EINTR is only a signal":
++    check interrupted(osError(p.EINTR, "Interrupted system call"))
++  test "any other OSError is a real failure":
++    check not interrupted(osError(p.ECONNRESET, "Connection reset by peer"))
++    check not interrupted(osError(p.EPIPE, "Broken pipe"))
++  test "and so is an error that is not an OSError at all":
++    check not interrupted(newException(ValueError, "nonsense"))
++  test "but the words count where the code was lost":
++    # `httpclient` catches an OSError in places and re-raises its text, which
++    # keeps the message and drops the code.
++    check interrupted(newException(IOError, "Interrupted system call"))
++
++suite "retrying":
++  test "a body that works runs once":
++    var runs = 0
++    retrying 3:
++      runs.inc
++    check runs == 1
++
++  test "one cut short by a signal is run again":
++    var runs = 0
++    retrying 3:
++      runs.inc
++      if runs < 3: raise osError(p.EINTR, "Interrupted system call")
++    check runs == 3
++
++  test "a real failure is raised at once, not retried":
++    var runs = 0
++    expect OSError:
++      retrying 5:
++        runs.inc
++        raise osError(p.ECONNREFUSED, "Connection refused")
++    check runs == 1
++
++  test "and a signal that never stops gives up rather than spinning":
++    var runs = 0
++    expect OSError:
++      retrying 4:
++        runs.inc
++        raise osError(p.EINTR, "Interrupted system call")
++    check runs == 4