git » sdk » commit 875abe9

Disconnect on start timeout

author Eric Roberts
2026-09-23 14:33:48 UTC
committer Stephen Paul Weber
2026-09-23 14:40:23 UTC
parent 6329d4e5753c531dd759a9fa8c21c742d3a2c583
Disconnect on start timeout

xmpp.start() has a 2 second timeout. However this doesn't mean that the
stream won't open, it could just be slow. Our existing code does a
connection-closed event on any error which transitions our state to
offline. So if it takes longer than two seconds to open the stream then
we transition to offline, and then our xmpp.on("online") listener gets
triggered, and then we try to call connection-success from offline,
which causes the error.

If we just did nothing in the case of TimeoutError (e.g. just return
instead of call disconnect) and it was just slow, then everything works.

However disconnect triggers xmpp's own connection retry logic, so I
think that maybe makes more sense. I'm open to suggestion. With the
explicit disconnect it's technically possible I think for the stream to
open while we're disconnecting which would mean we go:

- connecting
- online
- disconnected

but then it will just try to connect again (retry logic tries every
second) so I think that's fine.

I tested this by modifying code inside of xmpp to add a 3 second delay.
This line* in @xmpp/websocket/lib/Socket.js:

``` js
message: ({ data }) => this.emit("data", data),
```

was replaced with this:

``` js
message: ({ data }) => {
  if (!this.delayedInitialFrames) {
    this.delayedInitialFrames = [];

    setTimeout(() => {
      const frames = this.delayedInitialFrames;
      this.initialDelayDone = true;

      for (const frame of frames) {
	this.emit("data", frame);
      }
    }, 3000);
  }

  if (this.initialDelayDone) {
    this.emit("data", data);
  } else {
    this.delayedInitialFrames.push(data);
  }
},
````

which let me see the timeout error actually occur in a reproducible
fashion and confirm that this code change fixes it.

I also increased our timeout to 10 seconds because 2 seconds seems very
short. So that should reduce the chances of this happening at all in the
first place.

* https://github.com/xmppjs/xmpp.js/blob/main/packages/websocket/lib/Socket.js#L29
borogove/streams/XmppJsStream.hx +15 -4
test/TestAll.hx +3 -0
test/TestXmppJsStream.hx +44 -0 
diff --git a/borogove/streams/XmppJsStream.hx b/borogove/streams/XmppJsStream.hx
index 84665fd..0b5829c 100644
--- a/borogove/streams/XmppJsStream.hx
+++ b/borogove/streams/XmppJsStream.hx
@@ -24,6 +24,7 @@ extern class XmppJsClient {
 	function new(options:Dynamic);
 	function start():Promise<Dynamic>;
 	function stop():Promise<Dynamic>;
+	function disconnect():Promise<Dynamic>;
 	function on(eventName:String, callback:(Dynamic)->Void):Void;
 	function send(stanza:XmppJsXml):Void;
 	final NS:String;
@@ -144,6 +145,7 @@ extern class XmppJsStreamFeatures {
 }
 
 class XmppJsStream extends GenericStream {
+	private final createClient:Dynamic->XmppJsClient;
 	private var client:XmppJsClient;
 	private var jid:XmppJsJID;
 	private var debug = js.Browser.getLocalStorage()?.getItem("BOROGOVE_XMPP_DEBUG") == "1" || js.Syntax.code("globalThis.process?.env?.BOROGOVE_XMPP_DEBUG") == "1";
@@ -155,8 +157,9 @@ class XmppJsStream extends GenericStream {
 	private var resumed = false;
 	private var everConnected = false;
 
-	override public function new() {
+	override public function new(?createClient:Dynamic->XmppJsClient) {
 		super();
+		this.createClient = createClient ?? (options -> new XmppJsClient(options));
 		state = new FSM({
 			transitions: [
 				{ name: "connect-requested", from: ["offline"], to: "connecting" },
@@ -246,7 +249,7 @@ class XmppJsStream extends GenericStream {
 
 		final clientId = jid.resource;
 		var credentials: Dynamic = null;
-		final xmpp = new XmppJsClient({
+		final xmpp = createClient({
 			service: jid.domain,
 			resource: jid.resource,
 			credentials: (callback, mechanisms: Dynamic, fast: Null<{mechanism: String}>) -> {
@@ -273,7 +276,8 @@ class XmppJsStream extends GenericStream {
 					credentials = creds;
 					return callback(creds, creds.mechanism ?? mech, new XmppJsXml("user-agent", { id: clientId }));
 				});
-			}
+			},
+			timeout: 10_000,
 		});
 		new XmppJsScramSha1(xmpp.saslFactory);
 		xmpp.jid = this.jid;
@@ -381,7 +385,14 @@ class XmppJsStream extends GenericStream {
 
 		resumed = false;
 		xmpp.start().catchError(function (err) {
-			trace(err);
+			if (err.name == 'TimeoutError') {
+				trace("XmppJsStream: Timed out, disconnecting.");
+				xmpp.disconnect();
+				return;
+			}
+
+			trace("XmppJsStream: Error on start", err);
+
 			if (this.state.can("connection-error")) this.state.event("connection-error", { error: err });
 		});
 	}
diff --git a/test/TestAll.hx b/test/TestAll.hx
index 707b619..df0c519 100644
--- a/test/TestAll.hx
+++ b/test/TestAll.hx
@@ -37,6 +37,9 @@ class TestAll {
 			new TestStringUtil(),
 			new TestUtil(),
 			new TestXEP0393(),
+#if js
+			new TestXmppJsStream(),
+#end
 #if eval
 			new TestCaps(),
 #else
diff --git a/test/TestXmppJsStream.hx b/test/TestXmppJsStream.hx
new file mode 100644
index 0000000..f40f235
--- /dev/null
+++ b/test/TestXmppJsStream.hx
@@ -0,0 +1,44 @@
+package test;
+
+import borogove.streams.XmppJsStream;
+import js.lib.Promise;
+import utest.Assert;
+import utest.Async;
+
+@:access(borogove.streams.XmppJsStream)
+class TestXmppJsStream extends utest.Test {
+	public function testTimeoutDisconnects(async:Async) {
+		final createFakeClient = (options:Dynamic) -> {
+			final client = new XmppJsClient(options);
+			Reflect.setField(
+				client,
+				"start",
+				() -> Promise.reject(timeoutError())
+			);
+			Reflect.setField(
+				client,
+				"disconnect",
+				() -> {
+					// I just want to check that we called disconnect.
+					// async.done() isn't sufficient by itself because
+					// my test needs at least one assertion.
+					Assert.isTrue(true);
+					async.done();
+					return Promise.resolve(null);
+				}
+			);
+
+			return client;
+		};
+
+		final stream = new XmppJsStream(createFakeClient);
+		stream.connect("test@example.com/test", null);
+	}
+
+	private function timeoutError() {
+		final error = new js.lib.Error();
+		error.name = "TimeoutError";
+
+		return error;
+	}
+}