Repository navigation
Ergo push notification fixes, with #10's quit-log change - #18
Closed
luizribeiro wants to merge 4 commits into
Closed
luizribeiro wants to merge 4 commits into
luizribeiro wants to merge 4 commits into
Conversation
When a blocking write to a dropped connection fails, Ergo logs a "quit" line with the socket error, and Go's error text carries both ends of the connection: "send error to client : <nick> : write tcp <server>-><client>: write: broken pipe". Production logged five of these, against our promise not to keep members' addresses. Exclude the quit log type, as connect-ip already is. In Ergo 2.19.1 the only other quit line is "<nick> is no longer on the server", which names no address but is itself a record of when members leave, which we have no use for. Patching the address out of the error would keep a line nobody reads. The ergo VM test now has a client reset its TLS connection mid-burst: it sends 20 PINGs, waits for the first PONG, then closes with SO_LINGER 0. Fakelag holds the remaining replies past the RST, so Ergo's next write fails. Without the fix the subtest fails on that line; irc-client gains --close=reset for it.
Ergo v2.19.1 sends every push at high urgency, ignoring the urgency each caller picks. Upstream PR #2471 (merged 2026-10-22, in no release yet) passes it through, so maintenance PINGs and MARKREADs go out at normal urgency and stop waking phones; messages and the registration probe stay high. The commit applies to v2.19.1 unchanged. Upstream added no tests; Ergo's own suite still runs in the build.
The ergo test will need a certificate from a test CA too, as the mail node has.
Each browser or phone a member turns notifications on in takes a subscription, and Ergo's default of four runs out quickly for anyone with a few devices and a beta client besides. The ergo VM test registers eleven endpoints and sees ten accepted and the eleventh refused. Ergo probes each endpoint with a push before accepting it, over HTTPS and only to a public address, so the client serves one at 203.0.113.7 with a certificate credenza trusts.
This was referenced Nov 3, 2026
Merged
Owner
Author
|
Landed in main via #19 (rebased). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bundles Ergo's changes so they ship in a single Ergo restart. Includes #10's commit (rebased onto main) and supersedes #10.
fetchpatch, and it applies to v2.19.1 unchanged. Maintenance PINGs and MARKREADs now go out at normal urgency, while messages and the registration probe stay at high. Upstream added no tests for it; Ergo's owngo testsuite still runs in the build and passes.webpush.max-subscriptions = 10(the default is 4).delaystays unset.tests/e2e/lib/certificates.nix(mail and ergo both use it)Test: the new
ergosubtest sends 11WEBPUSH REGISTERrequests. It checks that 10 are accepted and the 11th getsMAX_REGISTRATIONS. Ergo sends a real probe push to each endpoint before accepting it, and only over HTTPS to a public address. So intests/e2e/ergo.nixthe client runs nginx on 203.0.113.7 aspush.test, answering 201 with a test-CA certificate that credenza trusts.Run locally:
just prod build, plus the ergo, security and webchat VM tests.