| Commit message (Collapse) | Author | Age |
|---|
| |
|
|
|
|
|
|
| |
This was actually two issues in one!
Issue 1: `isRetryable` did not consider whether we got a response or not. It assumed that the presence of a request in the error signaled that the error was definitely due to network issues, when in fact it's the presence of a request _and_ the absence of a response. That's my misreading of the Axios docs; the replacement `isRetryable` is more thorough.
Issue 2: operations in the outbox queue that fail with an exception stop the outbox drain process from making further progress, _and_ they stay in the queue. The outbox now dequeues jobs that throw an exception, and restarts itself if it terminates with a non-empty queue. The code that does this is _heinous_, but it seems to work well enough… Words I'm sure I won't come to regret.
|
| |
|
|
|
|
|
|
|
|
| |
A handful of operations are "synchronized" - that is, the server sends back information about them when the client asks to perform them, but notifies _all_ clients of completion through the event stream. As of this writing, these operations include sending and deleting messages, creating and deleting channels, and anything that creates new users.
We can use the outbox for most of these. I've opted _not_ to use the outbox for creating users, as that often takes place when the client is not connected to the event stream (and can't be) and so cannot discover that the operation has been completed after it is sent.
Outboxed tasks are objects, not closures, even though they behave in closure-like ways (`send()` carries out what amounts to "attempt this operation until it succeeds"). This is deliberate; I want the properties of incomplete tasks to be inspectable down the line, so that we can put them in the UI. If they're mere closures, we can't do that.
It is deliberate that `outbox.postToChannel` et al do not return promises. I contemplated it, but it will interact weirdly with outbox serialization, when we get to that. Rather than relying on a promise to determine when an operation has completed, clients of `Outbox` should monitor its properties. (We can add additional view properties to make that easier.)
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is in lieu of saving the outbox. I tried that, and:
* If messages are dropped from the saved outbox before calling `api.postToChannel`, then messages "in flight" are lost when the page is reloaded unless the send succeeds after the client vanishes, as they are not re-sent when the page loads.
* If messages are dropped from the saved outbox after calling `api.postToChannel`, then messages "in flight" are duplicated when the page is reloaded and they get re-sent.
The CAP theorem is real and can hurt you.
The appropriate compensating mechanism would be a client-generated per-operation nonce, with server-side support for replaying responses by nonce if an operation already completed. That's a pretty big undertaking, and it's one we should probably do, but it's larger than I want to take on right now. Instead, we warn the user, and they can make their own decision.
Except we don't, sometimes.
When the client runs in a browser, this event handler prompts the user for confirmation before reloading, navigating away, closing the tab, or quitting. When run in a Safari app container, though, it only warns before reloading. Closing the window or quitting the app do not provoke a prompt. The warning is "best" effort. The failure mode is lost messages, which isn't particularly best.
|
| |
|
|
|
|
|
|
| |
directly from the UI.
This primarily serves to free up the message input immediately, so that the user can start drafting their next message right away. The wait while a message is being sent is actively disruptive when using Pilcrow on a server with noticable latency (hi.grimoire.ca has around 700ms), and this largely alleviates it.
Unsent messages can be lost if the client is closed or deactivated before they make it to the head of the queue.
|
| |\
| |
| |
| | |
Merged in spite of misgivings. This method will loop over the request until it completes, even if the user moves to a view where the response is no longer relevant.
|
| | | |
|
| | |
| |
| |
| |
| |
| | |
This is intended to transparently resume the session (using `boot` to start over) after more serious connection interruptions. It interacts with the heartbeat timeout: we let the browser try to reconnect through `EventSource` on its own for up to 30 seconds, before intervening, closing the event source, and starting attempts to call `boot`.
This covers both initial boot, which will now hang if the server is unavailable (sorry), and reconnection after an event timeout. No other operations are retried (particularly, sending a message is _not_ retried).
|
| |\ \
| | |
| | |
| | | |
This turned out to be a bug.
|
| |/ /
| |
| |
| | |
This was always intended, but it wasn't working that way because `channelsMeta.get(id)?.lastReadAt` evaluates to `undefined`, not `null`. Strict equality (`===`) treats those as distinct values.
|
| | | | |
| | \ | |
| |\ \ \
| |_|/
|/| |
| | |
| | | |
* Open all outbound links from messages in new tabs/windows.
* Stop user agents from sending referer headers pointing to Pilcrow instances when opening outbound links.
|
| | |/
|/|
| |
| |
| |
| |
| |
| | |
contrast.
We were ending up with colours picked by a more specific rule (`a, a:hover, a:visited, a:active`, from `app.css`), which was suppressing the colours we wanted. This is not a particularly elegant solution, but it is _an_ solution.
Code organized this way to avoid giving `a` elements a background colour.
|
| | | |
|
| |/
|
|
|
|
|
|
| |
* `target="_blank"`: when Pilcrow is running in a browser, clicking a link should not replace Pilcrow with the target of the link. Pilcrow is "app-like" enough that opening links in a new tab _by default_, without user intervention, is likely more appropriate.
* `rel="noreferrer"`, which (A) stops most UAs from setting a referrer header when following those links, and (B) also implies `noopener`, preventing the link target from using `window.opener` from reaching back into Pilcrow's DOM.
I briefly experimented with DOMPurify's `RETURN_DOM_FRAGMENT` mode, which would have made the tests somewhat easier to write, but I wasn't able to find a good way to integrate the returned `DocumentFragment` objects with Svelte components, so HTML-as-strings it is. Sigh.
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| | |
The changes introduced in the previous commit make it possible to detect lost connections and restart them, so do so. The process is pretty simple - a new remote state is spun up using `/api/boot`, swapped in for the existing state, and a `new EventSource` is started from that new remote state to consume events.
This can induce some anomalies. For example, messages that arrive on the server between the loss of one connection and the creation of the next one just "show up" in boot, without ever appearing in the event stream. (This is technically also true on client startup, but it's easier to expect in that situation.) This is something we'll need to consider when implementing things like notifications or unread flags, though the ones we have today, which are state-based, do work fine.
By design, this _does not_ retry either the `/api/boot` call or the new event source setup. Event sources will try to reconnect on their own, up to a point, so that's fine, but we need to build something more robust for `/api/boot`. I want to tackle that separately from detecting lost connections and reacting to them, but that does mean that this is not a complete solution to client reconnects.
|
| |/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A heartbeat is an event that the server synthesizes any time an event stream has been idle for longer than some timeout. They allow clients to detect disconnection and network problems, which would otherwise go unnoticed because event streams are a one-way channel. Most network problems only become clear when the offended party tries to _send_ something, and subscribing to an event stream only sends something during the request phase.
Technically, Pilcrow has always sent these, since we started using Axum's SSE support: it defaults to sending a dummy event after 15 seconds (consisting of `":\n\n"`, which is then ignored). I've built Pilcrow's heartbeat support out of that, by customizing the event sent back. The results _mostly_ look like existing events, but there are two key differences:
* Heartbeats don't have `id` fields in the event stream. They're synthetic, and they don't participate in either the "resume at" sequence management, or the last-event-id header-based resumption management.
* Heartbeats have an `event` but no `type` field in the message body. There are no subtypes.
To make it less likely that clients will race with the server on expiring timeouts, heartbeats are sent about five seconds early. In this change, heartbeats are due after 20 seconds, but are sent after 15. If it takes longer than five seconds for a heartbeat to arrive, a client can and should treat that as a network problem and reconnect, but I'd really like to avoid that happening over differences smaller than a second, so I've left a margin.
I originally sketched this out in conversation with @wlonk as having each event carry a deadline for the next one. I ultimately opted not to do that for a few reasons. First, Axum makes it hard - the built-in keep-alive support only works with a static event, and cannot make dynamic ones whose payloads might vary (for example if the deadline is variable). Second, it's complex, to no apparent gain, and adds deadline information to _every_ event type.
This implementation, instead, sends deadline information as part of boot, as a fixed interval in seconds. Clients are responsible for working out deadlines based on message arrivals. This is fine; heartbeat-based connection management is best effort at the best of times, so a few milliseconds of slop in either direction won't hurt anything.
The existing client ignores these events entirely, which is convenient.
The new heartbeat event type is defined alongside the main event type, to make it less likely that we'll inadvertently make changes to one but not the other. We can still do so advertently, I just don't want it to be an accident.
|
| | |
|
| |\ |
|
| | | |
|
| | |
| |
| |
| |
| |
| | |
357116366c1307bedaac6a3dfe9c5ed8e0e0c210 wasn't updated (and wasn't quite correct then, either).
I haven't found a way to derive it from the name of the type.
|
| |\ \ |
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | | |
|
| | |/ |
|
| |/ |
|
| |
|
|
| |
In a discussion with wlonk, we both agreed that 15 days is _too_ aggressive, but also that it's not quite time to implement configurable expiry.
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Sorry about the thousand-line omnibus change; this is functionally a rewrite of the client's state tracking, flavoured to resemble the existing code as far as is possible, rather than something that can be parted out and committed in pieces.
Highlights:
* No more `store.writeable()`s. All state is now tracked using state runs or derivatives. State is still largely structured the way it was, but several bits of nested state have been rewritten to ensure that their properties are reactive just as much as their containers are.
* State is no longer global. `(app)/+layout` manages a stateful session, created via its load hook and started/stopped via component mount and destroy events. The session also tracks an event source for the current state, and feeds events into the state, broadly along the same lines as the previous stores-based approach.
Together these two changes fix up several rough spots integrating state with Svelte, and allow for the possibility of multiple states. This is a major step towards restartable states, and thus towards better connection management, which will require the ability to "start over" once a connection is restored.
|
| |/ |
|
| |\ |
|
| | |
| |
| |
| | |
Only once on load, then once per new message.
|
| |/ |
|
| |\ |
|
| | | |
|
| | | |
|
| | | |
|
| | |\ |
|
| | | |
| | |
| | |
| | |
| | |
| | | |
the DOM.
Prevents this from breaking during DOM unmounting, when leaving a channel.
|
| | | |
| | |
| | |
| | | |
That no longer vitally pertains.
|
| | | |
| | |
| | |
| | | |
To facilitate PWA behaviour.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This is stored locally, and, while parallel to channel info, is not the
same as.
Eventually, this may hold info about moot/decayed channels, and grow
unbounded. That'll need to be addressed.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
When we hit the boot endpoint, we get the server's view of things. If we
just setChannels with that, we overwrite all our locally-stored info in
on things like lastReadAt. So we need to merge data.
Eventually, this might militate for a `meta` key containing an object of
locally stored data, rather than having to handle each key specially.
|
| |\ \ \
| |_|/
|/| | |
|
| |/ /
| |
| |
| |
| |
| |
| |
| | |
Access to "global" (maybe "external?") state is now handled at the top level of the component hierarchy, in `+page.svelte`, `+layout.svelte`, and their associated scripts. State is otherwise passed down through props, and changes are passed up through callbacks.
This is - hopefully - groundwork for refactoring state management a bit. I wanted to move access to state out to a smaller number of places, so that I have fewer places to update to implement reconnect logic. My broader goal is to make it easier to refactor these kinds of external side effects, as well, though no such changes are in this branch.
This change also makes testing a mile easier, since tests can interact with props and callbacks instead of emulating the whole HTTP request stack and the Pilcrow API. This change removes do-very-little tests.
|
| | | |
|
| | | |
|
