Propagate reason for chat disconnect to listeners

signalapp · Sep 5, 2024 · 40aaecb · 40aaecb
1 parent 823cfcf
commit 40aaecb
Show file tree

Hide file tree

Showing 14 changed files with 194 additions and 81 deletions.
diff --git a/java/shared/java/org/signal/libsignal/internal/NativeTesting.java b/java/shared/java/org/signal/libsignal/internal/NativeTesting.java
@@ -63,6 +63,7 @@ private NativeTesting() {}
   public static native Object TESTING_ChatServiceResponseAndDebugInfoConvert() throws Exception;
   public static native Object TESTING_ChatServiceResponseConvert(boolean bodyPresent) throws Exception;
   public static native void TESTING_ChatService_InjectConnectionInterrupted(long chat);
+  public static native void TESTING_ChatService_InjectIntentionalDisconnect(long chat);
   public static native void TESTING_ChatService_InjectRawServerRequest(long chat, byte[] bytes);
   public static native void TESTING_ErrorOnBorrowAsync(Object input);
   public static native CompletableFuture TESTING_ErrorOnBorrowIo(long asyncRuntime, Object input);

diff --git a/node/Native.d.ts b/node/Native.d.ts
@@ -3,6 +3,8 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 //
 
+import { LibSignalError } from './ts/Errors';
+
 // WARNING: this file was automatically generated
 
 type Uuid = Buffer;
@@ -120,7 +122,7 @@ export abstract class ChatListener {
     ack: ServerMessageAck
   ): void;
   _queue_empty(): void;
-  _connection_interrupted(): void;
+  _connection_interrupted(reason: LibSignalError | null): void;
 }
 
 export abstract class MakeChatListener extends ChatListener {}
@@ -498,6 +500,7 @@ export function TESTING_ChatServiceErrorConvert(errorDescription: string): void;
 export function TESTING_ChatServiceResponseAndDebugInfoConvert(): ResponseAndDebugInfo;
 export function TESTING_ChatServiceResponseConvert(bodyPresent: boolean): ChatResponse;
 export function TESTING_ChatService_InjectConnectionInterrupted(chat: Wrapper<Chat>): void;
+export function TESTING_ChatService_InjectIntentionalDisconnect(chat: Wrapper<Chat>): void;
 export function TESTING_ChatService_InjectRawServerRequest(chat: Wrapper<Chat>, bytes: Buffer): void;
 export function TESTING_ErrorOnBorrowAsync(_input: null): Promise<void>;
 export function TESTING_ErrorOnBorrowIo(asyncRuntime: Wrapper<NonSuspendingBackgroundThreadRuntime>, _input: null): Promise<void>;

diff --git a/node/ts/net.ts b/node/ts/net.ts
@@ -126,10 +126,11 @@ export interface ConnectionEventsListener {
   /**
    * Called when the client gets disconnected from the server.
    *
-   * This includes both deliberate disconnects as well as unexpected socket closures that will be
-   * automatically retried.
+   * This includes both deliberate disconnects as well as unexpected socket
+   * closures. If the closure was not due to a deliberate disconnect, the error
+   * will be provided.
    */
-  onConnectionInterrupted(): void;
+  onConnectionInterrupted(cause: LibSignalError | null): void;
 }
 
 export interface ChatServiceListener extends ConnectionEventsListener {
@@ -251,8 +252,8 @@ export class AuthenticatedChatService implements ChatService {
       _queue_empty(): void {
         listener.onQueueEmpty();
       },
-      _connection_interrupted(): void {
-        listener.onConnectionInterrupted();
+      _connection_interrupted(cause: LibSignalError | null): void {
+        listener.onConnectionInterrupted(cause);
       },
     };
     Native.ChatService_SetListenerAuth(
@@ -331,8 +332,8 @@ export class UnauthenticatedChatService implements ChatService {
       _queue_empty(): void {
         throw new Error('Event not supported on unauthenticated connection');
       },
-      _connection_interrupted(): void {
-        listener.onConnectionInterrupted();
+      _connection_interrupted(cause: LibSignalError | null): void {
+        listener.onConnectionInterrupted(cause);
       },
     };
     Native.ChatService_SetListenerUnauth(

diff --git a/node/ts/test/NetTest.ts b/node/ts/test/NetTest.ts
@@ -23,6 +23,7 @@ import {
 import { randomBytes } from 'crypto';
 import { ChatResponse } from '../../Native';
 import { CompletablePromise } from './util';
+import { fail } from 'assert';
 
 use(chaiAsPromised);
 use(sinonChai);
@@ -290,15 +291,23 @@ describe('chat service api', () => {
       INVALID_MESSAGE,
       INCOMING_MESSAGE_2,
     ];
-    const callsReceived: string[] = [];
-    const callsExpected: string[] = [
-      '_incoming_message',
-      '_queue_empty',
-      '_incoming_message',
-      '_connection_interrupted',
+    const callsReceived: [string, (object | null)[]][] = [];
+    const callsExpected: [string, ((value: object | null) => void)[]][] = [
+      ['_incoming_message', []],
+      ['_queue_empty', []],
+      ['_incoming_message', []],
+      [
+        '_connection_interrupted',
+        [
+          (error: object | null) =>
+            expect(error)
+              .instanceOf(LibSignalErrorBase)
+              .property('code', ErrorCode.IoError),
+        ],
+      ],
     ];
-    const recordCall = function (name: string) {
-      callsReceived.push(name);
+    const recordCall = function (name: string, ...args: (object | null)[]) {
+      callsReceived.push([name, args]);
       if (callsReceived.length == callsExpected.length) {
         completable.complete();
       }
@@ -314,8 +323,8 @@ describe('chat service api', () => {
       onQueueEmpty(): void {
         recordCall('_queue_empty');
       },
-      onConnectionInterrupted(): void {
-        recordCall('_connection_interrupted');
+      onConnectionInterrupted(cause: object | null): void {
+        recordCall('_connection_interrupted', cause);
       },
     };
     const chat = net.newAuthenticatedChatService('', '', false, listener);
@@ -327,7 +336,43 @@ describe('chat service api', () => {
     );
     Native.TESTING_ChatService_InjectConnectionInterrupted(chat.chatService);
     await completable.done();
-    expect(callsReceived).to.eql(callsExpected);
+
+    expect(callsReceived).to.have.lengthOf(callsExpected.length);
+    callsReceived.forEach((element, index) => {
+      const [call, args] = element;
+      const [expectedCall, expectedArgs] = callsExpected[index];
+      expect(call).to.eql(expectedCall);
+      expect(args.length).to.eql(expectedArgs.length);
+      args.map((arg, i) => {
+        expectedArgs[i](arg);
+      });
+    });
+  });
+
+  it('listener gets null cause for intentional disconnect', async () => {
+    const net = new Net(Environment.Staging, userAgent);
+    const completable = new CompletablePromise();
+    const connectionInterruptedReasons: (object | null)[] = [];
+    const listener: ChatServiceListener = {
+      onIncomingMessage(
+        _envelope: Buffer,
+        _timestamp: number,
+        _ack: ChatServerMessageAck
+      ): void {
+        fail('unexpected call');
+      },
+      onQueueEmpty(): void {
+        fail('unexpected call');
+      },
+      onConnectionInterrupted(cause: object | null): void {
+        connectionInterruptedReasons.push(cause);
+        completable.complete();
+      },
+    };
+    const chat = net.newAuthenticatedChatService('', '', false, listener);
+    Native.TESTING_ChatService_InjectIntentionalDisconnect(chat.chatService);
+    await completable.done();
+    expect(connectionInterruptedReasons).to.eql([null]);
   });
 
   it('client can respond with http status code to a server message', () => {

diff --git a/rust/bridge/node/bin/Native.d.ts.in b/rust/bridge/node/bin/Native.d.ts.in
@@ -3,6 +3,8 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 //
 
+import { LibSignalError } from './ts/Errors';
+
 // WARNING: this file was automatically generated
 
 type Uuid = Buffer;
@@ -120,7 +122,7 @@ export abstract class ChatListener {
     ack: ServerMessageAck
   ): void;
   _queue_empty(): void;
-  _connection_interrupted(): void;
+  _connection_interrupted(reason: LibSignalError | null): void;
 }
 
 export abstract class MakeChatListener extends ChatListener {}

diff --git a/rust/bridge/shared/testing/src/net.rs b/rust/bridge/shared/testing/src/net.rs
@@ -151,6 +151,7 @@ make_error_testing_enum! {
         AllConnectionRoutesFailed => AllConnectionRoutesFailed,
         ServiceInactive => ServiceInactive,
         ServiceUnavailable => ServiceUnavailable,
+        ServiceIntentionallyDisconnected => ServiceIntentionallyDisconnected,
     }
 }
 
@@ -185,6 +186,9 @@ fn TESTING_ChatServiceErrorConvert(
         }
         TestingChatServiceError::ServiceInactive => ChatServiceError::ServiceInactive,
         TestingChatServiceError::ServiceUnavailable => ChatServiceError::ServiceUnavailable,
+        TestingChatServiceError::ServiceIntentionallyDisconnected => {
+            ChatServiceError::ServiceIntentionallyDisconnected
+        }
     })
 }
 
@@ -278,6 +282,15 @@ fn TESTING_ChatService_InjectConnectionInterrupted(chat: &Chat) {
         .expect("not closed");
 }
 
+#[bridge_fn]
+fn TESTING_ChatService_InjectIntentionalDisconnect(chat: &Chat) {
+    chat.synthetic_request_tx
+        .blocking_send(chat::ws::ServerEvent::Stopped(
+            ChatServiceError::ServiceIntentionallyDisconnected,
+        ))
+        .expect("not closed");
+}
+
 #[bridge_fn(jni = false, ffi = false)]
 fn TESTING_ServerMessageAck_Create() -> ServerMessageAck {
     ServerMessageAck::new(Box::new(|_| Box::pin(std::future::ready(Ok(())))))

diff --git a/rust/bridge/shared/types/src/ffi/error.rs b/rust/bridge/shared/types/src/ffi/error.rs
@@ -75,34 +75,35 @@ pub enum SignalErrorCode {
     UsernameLinkInvalidEntropyDataLength = 127,
     UsernameLinkInvalid = 128,
 
-    UsernameDiscriminatorCannotBeEmpty = 140,
-    UsernameDiscriminatorCannotBeZero = 141,
-    UsernameDiscriminatorCannotBeSingleDigit = 142,
-    UsernameDiscriminatorCannotHaveLeadingZeros = 143,
-    UsernameDiscriminatorTooLarge = 144,
+    UsernameDiscriminatorCannotBeEmpty = 130,
+    UsernameDiscriminatorCannotBeZero = 131,
+    UsernameDiscriminatorCannotBeSingleDigit = 132,
+    UsernameDiscriminatorCannotHaveLeadingZeros = 133,
+    UsernameDiscriminatorTooLarge = 134,
 
-    IoError = 130,
+    IoError = 140,
     #[allow(dead_code)]
-    InvalidMediaInput = 131,
+    InvalidMediaInput = 141,
     #[allow(dead_code)]
-    UnsupportedMediaInput = 132,
+    UnsupportedMediaInput = 142,
 
-    ConnectionTimedOut = 133,
-    NetworkProtocol = 134,
-    RateLimited = 135,
-    WebSocket = 136,
-    CdsiInvalidToken = 137,
-    ConnectionFailed = 138,
-    ChatServiceInactive = 139,
+    ConnectionTimedOut = 143,
+    NetworkProtocol = 144,
+    RateLimited = 145,
+    WebSocket = 146,
+    CdsiInvalidToken = 147,
+    ConnectionFailed = 148,
+    ChatServiceInactive = 149,
+    ChatServiceIntentionallyDisconnected = 150,
 
-    SvrDataMissing = 150,
-    SvrRestoreFailed = 151,
-    SvrRotationMachineTooManySteps = 152,
+    SvrDataMissing = 160,
+    SvrRestoreFailed = 161,
+    SvrRotationMachineTooManySteps = 162,
 
-    AppExpired = 160,
-    DeviceDeregistered = 161,
+    AppExpired = 170,
+    DeviceDeregistered = 171,
 
-    BackupValidation = 170,
+    BackupValidation = 180,
 }
 
 pub trait UpcastAsAny {
@@ -546,6 +547,9 @@ impl FfiError for ChatServiceError {
             Self::ServiceInactive => "Chat service inactive".to_owned(),
             Self::AppExpired => "App expired".to_owned(),
             Self::DeviceDeregistered => "Device deregistered or delinked".to_owned(),
+            Self::ServiceIntentionallyDisconnected => {
+                "Chat service explicitly disconnected".to_owned()
+            }
         }
     }
 
@@ -567,6 +571,9 @@ impl FfiError for ChatServiceError {
             Self::ServiceInactive => SignalErrorCode::ChatServiceInactive,
             Self::AppExpired => SignalErrorCode::AppExpired,
             Self::DeviceDeregistered => SignalErrorCode::DeviceDeregistered,
+            Self::ServiceIntentionallyDisconnected => {
+                SignalErrorCode::ChatServiceIntentionallyDisconnected
+            }
         }
     }
 }