From 8a329eb962eb98e89b41708d92b3f9298e4c21e1 Mon Sep 17 00:00:00 2001
From: Owen Jacobson <owen@grimoire.ca>
Date: Fri, 2 May 2025 02:13:19 -0400
Subject: Warn the user before navigating away, when the outbox has messages in
 it.

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.
---
 ui/routes/(app)/+layout.svelte | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

(limited to 'ui/routes/(app)/+layout.svelte')

diff --git a/ui/routes/(app)/+layout.svelte b/ui/routes/(app)/+layout.svelte
index 1ba3fa9..116767d 100644
--- a/ui/routes/(app)/+layout.svelte
+++ b/ui/routes/(app)/+layout.svelte
@@ -14,7 +14,7 @@
   let gesture = null;
 
   const { data, children } = $props();
-  const { session } = data;
+  const { session, outbox } = data;
 
   onMount(session.begin.bind(session));
   onDestroy(session.end.bind(session));
@@ -89,8 +89,44 @@
   async function createChannel(name) {
     await api.createChannel(name);
   }
+
+  function onbeforeunload(event) {
+    if (outbox.pending.length > 0) {
+      // Prompt the user that they have unsaved (unsent) messages that may be lost.
+      event.preventDefault();
+    }
+  }
 </script>
 
+<!--
+  In theory, we [should be][bfcache-why] using an ephemeral event handler for this, rather than
+  leaving it hooked up at all times. Some browsers decide whether a page is eligible for
+  back/forward caching based on whether it has a beforeunload handler (among other factors), and
+  having the event handler registered can slow down navigation on those browsers by forcing a
+  network reload when the page could have been restored from memory.
+
+  Most browsers _apparently_ no longer use that criterion, but I would have been inclined to follow
+  the advice regardless as I don't feel up to the task of cataloguing which browsers it applies to.
+  Unfortunately, it wouldn't matter if we did: SvelteKit itself registers beforeunload handlers, so
+  (at least as of this writing) any work we do to try to dynamically register or unregister
+  beforeunload handlers dynamically state would be wasted effort.
+
+  For posterity, though, the appropriate code for doing so based on outbox state looks like
+
+      $effect(() => {
+        if (outbox.pending.length > 0) {
+          window.addEventListener('beforeunload', onbeforeunload);
+        }
+
+        return () => {
+          window.removeEventListener('beforeunload', onbeforeunload);
+        };
+      });
+
+  [bfcache-why]: https://web.dev/articles/bfcache#beforeunload-caution
+-->
+<svelte:window {onbeforeunload} />
+
 <svelte:head>
   <!-- TODO: unread count? -->
   <title>pilcrow</title>
-- 
cgit v1.2.3


From d11ff421de581a835f1645b22d1f7f4304640b0c Mon Sep 17 00:00:00 2001
From: Owen Jacobson <owen@grimoire.ca>
Date: Mon, 5 May 2025 19:51:58 -0400
Subject: Use the outbox for more than just message sends.

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.)
---
 ui/lib/outbox.svelte.js                   | 46 +++++++++++++++++++++++++++----
 ui/routes/(app)/+layout.svelte            |  2 +-
 ui/routes/(app)/ch/[channel]/+page.svelte |  5 ++--
 3 files changed, 44 insertions(+), 9 deletions(-)

(limited to 'ui/routes/(app)/+layout.svelte')

diff --git a/ui/lib/outbox.svelte.js b/ui/lib/outbox.svelte.js
index 0681f29..de7f80e 100644
--- a/ui/lib/outbox.svelte.js
+++ b/ui/lib/outbox.svelte.js
@@ -1,12 +1,36 @@
 import * as api from './apiServer.js';
 import * as md from './markdown.js';
 
-class Message {
+class PostToChannel {
   constructor(channel, body) {
     this.channel = channel;
     this.body = body;
     this.renderedBody = md.render(body);
   }
+
+  async send() {
+    return await api.retry(() => api.postToChannel(this.channel, this.body));
+  }
+}
+
+class DeleteMessage {
+  constructor(messageId) {
+    this.messageId = messageId;
+  }
+
+  async send() {
+    return await api.retry(() => api.deleteMessage(this.messageId));
+  }
+}
+
+class CreateChannel {
+  constructor(name) {
+    this.name = name;
+  }
+
+  async send() {
+    return await api.retry(() => api.createChannel(this.name));
+  }
 }
 
 export class Outbox {
@@ -20,11 +44,23 @@ export class Outbox {
     this.pending = pending;
   }
 
-  send(channel, body) {
-    this.pending.push(new Message(channel, body));
+  enqueue(operation) {
+    this.pending.push(operation);
     this.start();
   }
 
+  createChannel(name) {
+    this.enqueue(new CreateChannel(name));
+  }
+
+  postToChannel(channel, body) {
+    this.enqueue(new PostToChannel(channel, body));
+  }
+
+  deleteMessage(messageId) {
+    this.enqueue(new DeleteMessage(messageId));
+  }
+
   start() {
     if (this.sending) {
       return;
@@ -38,9 +74,9 @@ export class Outbox {
 
   async drain() {
     while (this.pending.length > 0) {
-      const { channel, body } = this.pending[0];
+      const operation = this.pending[0];
 
-      await api.retry(() => api.postToChannel(channel, body));
+      await operation.send();
       this.pending.shift();
     }
   }
diff --git a/ui/routes/(app)/+layout.svelte b/ui/routes/(app)/+layout.svelte
index 116767d..a4ae442 100644
--- a/ui/routes/(app)/+layout.svelte
+++ b/ui/routes/(app)/+layout.svelte
@@ -87,7 +87,7 @@
   });
 
   async function createChannel(name) {
-    await api.createChannel(name);
+    outbox.createChannel(name);
   }
 
   function onbeforeunload(event) {
diff --git a/ui/routes/(app)/ch/[channel]/+page.svelte b/ui/routes/(app)/ch/[channel]/+page.svelte
index 9506b67..abec00d 100644
--- a/ui/routes/(app)/ch/[channel]/+page.svelte
+++ b/ui/routes/(app)/ch/[channel]/+page.svelte
@@ -4,7 +4,6 @@
   import ActiveChannel from '$lib/components/ActiveChannel.svelte';
   import MessageInput from '$lib/components/MessageInput.svelte';
   import { runs } from '$lib/runs.js';
-  import * as api from '$lib/apiServer';
 
   const { data } = $props();
   const { session, outbox } = data;
@@ -65,11 +64,11 @@
   }
 
   async function sendMessage(message) {
-    outbox.send(channel, message);
+    outbox.postToChannel(channel, message);
   }
 
   async function deleteMessage(id) {
-    await api.deleteMessage(id);
+    outbox.deleteMessage(id);
   }
 </script>
 
-- 
cgit v1.2.3

