Refact native application connection closing

taneltm · mrts · commit 1311000d4ae8 · 2025-01-06T09:55:26.000+02:00
Signed-off-by: Tanel Metsar &lt;taneltm@users.noreply.github.com&gt;
diff --git a/src/background-safari/services/NativeAppService.ts b/src/background-safari/services/NativeAppService.ts
@@ -52,7 +52,14 @@ export default class NativeAppService {
 
     try {
       const message = await new Promise<{ version: string }>((resolve, reject) => {
-        setTimeout(reject, libraryConfig.NATIVE_APP_HANDSHAKE_TIMEOUT);
+        setTimeout(
+          () => {
+            reject(new NativeUnavailableError(
+              `native application handshake timeout, ${libraryConfig.NATIVE_APP_HANDSHAKE_TIMEOUT}ms`
+            ));
+          },
+          libraryConfig.NATIVE_APP_HANDSHAKE_TIMEOUT,
+        );
 
         browser.runtime.sendNativeMessage(
           "application.id",
@@ -91,7 +98,7 @@ export default class NativeAppService {
     this.pending = null;
   }
 
-  async send<T>(message: NativeRequest): Promise<T> {
+  async send<T>(message: NativeRequest, timeout: number, throwAfterTimeout: Error): Promise<T> {
     if (message.command == "quit") return {} as T;
 
     switch (this.state) {
@@ -101,6 +108,8 @@ export default class NativeAppService {
         const sendPromise = new Promise<T>((resolve, reject) => {
           this.pending = { resolve, reject };
 
+          setTimeout(() => { reject(throwAfterTimeout); }, timeout );
+
           const onResponse = (message: any): void => {
             this.pending = null;
 
diff --git a/src/background/actions/TokenSigning/getCertificate.ts b/src/background/actions/TokenSigning/getCertificate.ts
@@ -34,7 +34,6 @@ import NativeAppService from "../../services/NativeAppService";
 import config from "../../../config";
 import errorToResponse from "./errorToResponse";
 import threeLetterLanguageCodes from "./threeLetterLanguageCodes";
-import { throwAfterTimeout } from "../../../shared/utils/timing";
 import tokenSigningResponse from "../../../shared/tokenSigningResponse";
 
 export default async function getCertificate(
@@ -74,10 +73,11 @@ export default async function getCertificate(
       },
     };
 
-    const response = await Promise.race([
-      nativeAppService.send<NativeGetSigningCertificateResponse>(message),
-      throwAfterTimeout(config.TOKEN_SIGNING_USER_INTERACTION_TIMEOUT, new UserTimeoutError()),
-    ]);
+    const response = await nativeAppService.send<NativeGetSigningCertificateResponse>(
+      message,
+      config.TOKEN_SIGNING_USER_INTERACTION_TIMEOUT,
+      new UserTimeoutError(),
+    );
 
     if (!response?.certificate) {
       return tokenSigningResponse<TokenSigningErrorResponse>("no_certificates", nonce);
diff --git a/src/background/actions/TokenSigning/sign.ts b/src/background/actions/TokenSigning/sign.ts
@@ -34,7 +34,6 @@ import NativeAppService from "../../services/NativeAppService";
 import config from "../../../config";
 import errorToResponse from "./errorToResponse";
 import threeLetterLanguageCodes from "./threeLetterLanguageCodes";
-import { throwAfterTimeout } from "../../../shared/utils/timing";
 import tokenSigningResponse from "../../../shared/tokenSigningResponse";
 
 
@@ -128,10 +127,11 @@ export default async function sign(
       },
     };
 
-    const response = await Promise.race([
-      nativeAppService.send<NativeSignResponse>(message),
-      throwAfterTimeout(config.TOKEN_SIGNING_USER_INTERACTION_TIMEOUT, new UserTimeoutError()),
-    ]);
+    const response = await nativeAppService.send<NativeSignResponse>(
+      message,
+      config.TOKEN_SIGNING_USER_INTERACTION_TIMEOUT,
+      new UserTimeoutError(),
+    );
 
     if (!response?.signature) {
       return tokenSigningResponse<TokenSigningErrorResponse>("technical_error", nonce);
diff --git a/src/background/actions/TokenSigning/status.ts b/src/background/actions/TokenSigning/status.ts
@@ -20,17 +20,19 @@
  * SOFTWARE.
  */
 
-import { NativeQuitRequest } from "@web-eid.js/models/message/NativeRequest";
+import UnknownError from "@web-eid.js/errors/UnknownError";
 
 import {
   TokenSigningErrorResponse,
   TokenSigningStatusResponse,
 } from "../../../models/TokenSigning/TokenSigningResponse";
 
 import NativeAppService from "../../services/NativeAppService";
+import config from "../../../config";
 import errorToResponse from "./errorToResponse";
 import tokenSigningResponse from "../../../shared/tokenSigningResponse";
 
+
 export default async function status(
   nonce: string,
 ): Promise<TokenSigningStatusResponse | TokenSigningErrorResponse> {
@@ -46,12 +48,11 @@ export default async function status(
       throw new Error("missing native application version");
     }
 
-    const message: NativeQuitRequest = {
-      command:   "quit",
-      arguments: {},
-    };
-
-    await nativeAppService.send(message);
+    await nativeAppService.send(
+      { command: "quit", arguments: {} },
+      config.NATIVE_GRACEFUL_DISCONNECT_TIMEOUT,
+      new UnknownError("native application failed to close gracefully"),
+    );
 
     return tokenSigningResponse<TokenSigningStatusResponse>("ok", nonce, { version });
   } catch (error) {
diff --git a/src/background/actions/authenticate.ts b/src/background/actions/authenticate.ts
@@ -32,7 +32,6 @@ import UnknownError from "@web-eid.js/errors/UnknownError";
 import actionErrorHandler from "../../shared/actionErrorHandler";
 import config from "../../config";
 import { getSenderUrl } from "../../shared/utils/sender";
-import { throwAfterTimeout } from "../../shared/utils/timing";
 
 export default async function authenticate(
   challengeNonce: string,
@@ -62,11 +61,11 @@ export default async function authenticate(
       },
     };
 
-    const response = await Promise.race([
-      nativeAppService.send<NativeAuthenticateResponse>(message),
-
-      throwAfterTimeout(userInteractionTimeout, new UserTimeoutError()),
-    ]);
+    const response = await nativeAppService.send<NativeAuthenticateResponse>(
+      message,
+      userInteractionTimeout,
+      new UserTimeoutError(),
+    );
 
     config.DEBUG && console.log("Authenticate: authentication token received");
 
diff --git a/src/background/actions/getSigningCertificate.ts b/src/background/actions/getSigningCertificate.ts
@@ -36,7 +36,6 @@ import NativeAppService from "../services/NativeAppService";
 import actionErrorHandler from "../../shared/actionErrorHandler";
 import config from "../../config";
 import { getSenderUrl } from "../../shared/utils/sender";
-import { throwAfterTimeout } from "../../shared/utils/timing";
 
 export default async function getSigningCertificate(
   sender: MessageSender,
@@ -63,10 +62,11 @@ export default async function getSigningCertificate(
       },
     };
 
-    const response = await Promise.race([
-      nativeAppService.send<NativeGetSigningCertificateResponse>(message),
-      throwAfterTimeout(userInteractionTimeout, new UserTimeoutError()),
-    ]);
+    const response = await nativeAppService.send<NativeGetSigningCertificateResponse>(
+      message,
+      userInteractionTimeout,
+      new UserTimeoutError(),
+    );
 
     const isResponseValid = (
       response?.certificate &&
diff --git a/src/background/actions/sign.ts b/src/background/actions/sign.ts
@@ -36,7 +36,6 @@ import NativeAppService from "../services/NativeAppService";
 import actionErrorHandler from "../../shared/actionErrorHandler";
 import config from "../../config";
 import { getSenderUrl } from "../../shared/utils/sender";
-import { throwAfterTimeout } from "../../shared/utils/timing";
 
 
 export default async function sign(
@@ -71,10 +70,11 @@ export default async function sign(
       },
     };
 
-    const response = await Promise.race([
-      nativeAppService.send<NativeSignResponse>(message),
-      throwAfterTimeout(userInteractionTimeout, new UserTimeoutError()),
-    ]);
+    const response = await nativeAppService.send<NativeSignResponse>(
+      message,
+      userInteractionTimeout,
+      new UserTimeoutError(),
+    );
 
     const isResponseValid = (
       response?.signature &&
diff --git a/src/background/actions/status.ts b/src/background/actions/status.ts
@@ -27,6 +27,7 @@ import {
 } from "@web-eid.js/models/message/ExtensionResponse";
 
 import Action from "@web-eid.js/models/Action";
+import UnknownError from "@web-eid.js/errors/UnknownError";
 import VersionMismatchError from "@web-eid.js/errors/VersionMismatchError";
 import { serializeError } from "@web-eid.js/utils/errorSerializer";
 
@@ -48,10 +49,11 @@ export default async function status(libraryVersion: string): Promise<ExtensionS
         : status.version
     );
 
-    await nativeAppService.send({
-      command:   "quit",
-      arguments: {},
-    });
+    await nativeAppService.send(
+      { command: "quit", arguments: {} },
+      config.NATIVE_GRACEFUL_DISCONNECT_TIMEOUT,
+      new UnknownError("native application failed to close gracefully"),
+    );
 
     const componentVersions = {
       library:   libraryVersion,
diff --git a/src/background/services/NativeAppService.ts b/src/background/services/NativeAppService.ts
@@ -21,20 +21,13 @@
  */
 
 import NativeUnavailableError from "@web-eid.js/errors/NativeUnavailableError";
-import UnknownError from "@web-eid.js/errors/UnknownError";
 import { deserializeError } from "@web-eid.js/utils/errorSerializer";
 import libraryConfig from "@web-eid.js/config";
 
-import { NativeFailureResponse } from "@web-eid.js/models/message/NativeResponse";
 import { NativeRequest } from "@web-eid.js/models/message/NativeRequest";
 import { Port } from "../../models/Browser/Runtime";
 import calculateJsonSize from "../../shared/utils/calculateJsonSize";
 import config from "../../config";
-import { throwAfterTimeout } from "../../shared/utils/timing";
-
-type UnwrappedPromise
-  = { resolve: (value?: any) => void; reject: (reason?: any) => void }
-  | null;
 
 export enum NativeAppState {
   UNINITIALIZED,
@@ -47,21 +40,23 @@ export default class NativeAppService {
   public state: NativeAppState = NativeAppState.UNINITIALIZED;
 
   private port: Port | null = null;
-  private pending: UnwrappedPromise = null;
-  private activeConnection: UnwrappedPromise = null;
 
   async connect(): Promise<{ version: string }> {
     this.state = NativeAppState.CONNECTING;
+    this.port  = browser.runtime.connectNative(config.NATIVE_APP_NAME);
 
-    this.port = browser.runtime.connectNative(config.NATIVE_APP_NAME);
     this.port.onDisconnect.addListener(this.disconnectListener.bind(this));
 
     try {
-      const message = await this.nextMessage(libraryConfig.NATIVE_APP_HANDSHAKE_TIMEOUT);
+      const message = await this.nextMessage(
+        libraryConfig.NATIVE_APP_HANDSHAKE_TIMEOUT,
+        new NativeUnavailableError(
+          `native application handshake timeout, ${libraryConfig.NATIVE_APP_HANDSHAKE_TIMEOUT}ms`
+        ),
+      );
 
       if (message.version) {
         this.state = NativeAppState.CONNECTED;
-        new Promise((resolve, reject) => this.activeConnection = { resolve, reject });
 
         return message;
       }
@@ -90,85 +85,50 @@ export default class NativeAppService {
     }
   }
 
-  async disconnectListener(): Promise<void> {
-    config.DEBUG && console.log("Native app disconnected");
+  disconnectListener(): void {
+    config.DEBUG && console.log("Native app disconnected.");
+
     // Accessing lastError when it exists stops chrome from throwing it unnecessarily.
     chrome?.runtime?.lastError;
 
-    // Defer active connection cleanup for Edge
-    await new Promise((resolve) => setTimeout(resolve));
-
-    this.activeConnection?.resolve();
     this.state = NativeAppState.DISCONNECTED;
-
-    this.pending?.reject?.(new UnknownError("native application closed the connection before a response"));
-    this.pending = null;
-  }
-
-  disconnectForcefully(): void {
-    this.state = NativeAppState.DISCONNECTED;
-
-    // At this point, requests should already be resolved.
-    // Rejecting a resolved promise is a NOOP.
-    this.pending?.reject?.(new UnknownError("extension closed connection to native app prematurely"));
-    this.pending = null;
-
-    this.port?.disconnect();
   }
 
   close(): void {
     if (this.state == NativeAppState.DISCONNECTED) return;
 
-    this.disconnectForcefully();
+    this.state = NativeAppState.DISCONNECTED;
+    this.port?.disconnect();
+
+    config.DEBUG && console.log("Native app port closed by extension.");
   }
 
-  send<T>(message: NativeRequest): Promise<T> {
+  async send<T>(message: NativeRequest, timeout: number, throwAfterTimeout: Error): Promise<T> {
     switch (this.state) {
       case NativeAppState.CONNECTED: {
-        return new Promise((resolve, reject) => {
-          this.pending = { resolve, reject };
-
-          const onResponse = async (message: T): Promise<void> => {
-            this.port?.onMessage.removeListener(onResponse);
+        config.DEBUG && console.log("Sending message to native app", JSON.stringify(message));
 
-            try {
-              await Promise.race([
-                this.activeConnection,
-                throwAfterTimeout(
-                  config.NATIVE_GRACEFUL_DISCONNECT_TIMEOUT,
-                  new Error("Native application did not disconnect after response")
-                ),
-              ]);
+        const messageSize = calculateJsonSize(message);
 
-            } catch (error) {
-              console.error(error);
-              this.disconnectForcefully();
-
-            } finally {
-              const error = (message as unknown as NativeFailureResponse)?.error;
-
-              if (error) {
-                reject(deserializeError(error));
-              } else {
-                resolve(message);
-              }
+        if (messageSize > config.NATIVE_MESSAGE_MAX_BYTES) {
+          throw new Error(`native application message exceeded ${config.NATIVE_MESSAGE_MAX_BYTES} bytes`);
+        }
 
-              this.pending = null;
-            }
-          };
+        this.port?.postMessage(message);
 
-          this.port?.onMessage.addListener(onResponse);
+        try {
+          const response = await this.nextMessage(timeout, throwAfterTimeout);
 
-          config.DEBUG && console.log("Sending message to native app", JSON.stringify(message));
+          this.close();
 
-          const messageSize = calculateJsonSize(message);
+          return response;
+        } catch (error) {
+          console.error(error);
 
-          if (messageSize > config.NATIVE_MESSAGE_MAX_BYTES) {
-            throw new Error(`native application message exceeded ${config.NATIVE_MESSAGE_MAX_BYTES} bytes`);
-          }
+          this.close();
 
-          this.port?.postMessage(message);
-        });
+          throw error;
+        }
       }
 
       case NativeAppState.UNINITIALIZED: {
@@ -197,7 +157,7 @@ export default class NativeAppService {
     }
   }
 
-  nextMessage(timeout: number): Promise<any> {
+  nextMessage(timeout: number, throwAfterTimeout: Error): Promise<any> {
     return new Promise((resolve, reject) => {
       let cleanup: (() => void) | null = null;
       let timer: ReturnType<typeof setTimeout> | null = null;
@@ -227,9 +187,7 @@ export default class NativeAppService {
       timer = setTimeout(
         () => {
           cleanup?.();
-          reject(new NativeUnavailableError(
-            `a message from native application was expected, but message wasn't received in ${timeout}ms`
-          ));
+          reject(throwAfterTimeout);
         },
         timeout,
       );
