Merge pull request #88 from MatrixAI/revert-86-feature-socket-errors

tegefaulkes · web-flow · commit e8b1d8a6d4f0 · 2024-02-06T15:53:33.000+11:00
Revert "Gracefully handle certain `QUICSocket.send` errors"
diff --git a/src/QUICClient.ts b/src/QUICClient.ts
@@ -247,34 +247,8 @@ class QUICClient {
     // the client, because the client bridges the push flow from the connection
     // to the socket.
     socket.connectionMap.set(connection.connectionId, connection);
-    // Set up intermediate abort signal
-    const abortController = new AbortController();
-    const abortHandler = () => {
-      abortController.abort(ctx.signal.reason);
-    };
-    if (ctx.signal.aborted) abortController.abort(ctx.signal.reason);
-    else ctx.signal.addEventListener('abort', abortHandler);
-    const handleEventQUICClientErrorSend = (
-      evt: events.EventQUICClientErrorSend,
-    ) => {
-      // @ts-ignore: the error contains `code` but not part of the type
-      if (evt.detail.code === 'EINVAL') {
-        abortController.abort(
-          new errors.ErrorQUICClientInvalidArgument(undefined, {
-            cause: evt.detail,
-          }),
-        );
-      }
-    };
-    client.addEventListener(
-      `${events.EventQUICClientErrorSend.name}-${connection.sendId}`,
-      handleEventQUICClientErrorSend,
-    );
     try {
-      await connection.start(undefined, {
-        timer: ctx.timer,
-        signal: abortController.signal,
-      });
+      await connection.start(undefined, ctx);
     } catch (e) {
       socket.connectionMap.delete(connection.connectionId);
       socket.removeEventListener(
@@ -310,12 +284,6 @@ class QUICClient {
         client.handleEventQUICClientClose,
       );
       throw e;
-    } finally {
-      ctx.signal.removeEventListener('abort', abortHandler);
-      client.removeEventListener(
-        `${events.EventQUICClientErrorSend.name}-${connection.sendId}`,
-        handleEventQUICClientErrorSend,
-      );
     }
     address = utils.buildAddress(host_, port);
     logger.info(`Created ${this.name} to ${address}`);
@@ -331,10 +299,6 @@ class QUICClient {
   protected config: Config;
   protected _closed: boolean = false;
   protected resolveClosedP: () => void;
-  /**
-   * Flag used to make sure network fail warnings are only logged once per failure
-   */
-  protected networkWarned: boolean = false;
 
   /**
    * Handles `EventQUICClientError`.
@@ -494,49 +458,15 @@ class QUICClient {
         evt.detail.port,
         evt.detail.address,
       );
-      this.networkWarned = false;
     } catch (e) {
-      switch (e.code) {
-        case 'EINVAL':
-          {
-            this.dispatchEvent(
-              new events.EventQUICClientErrorSend(
-                `${events.EventQUICClientErrorSend.name}-${evt.detail.id}`,
-                {
-                  detail: e,
-                },
-              ),
-            );
-          }
-          break;
-        case 'ENETUNREACH':
-          {
-            // We consider this branch a temp failure.
-            // For these error codes we rely on the connection's timeout to handle.
-            if (!this.networkWarned) {
-              this.logger.warn(
-                `client send failed with 'ENETUNREACH', likely due to network failure`,
-              );
-              this.networkWarned = true;
-            }
-          }
-          break;
-        default:
-          {
-            this.dispatchEvent(
-              new events.EventQUICClientError({
-                detail: new errors.ErrorQUICClientInternal(
-                  'Failed to send data on the QUICSocket',
-                  {
-                    data: evt.detail,
-                    cause: e,
-                  },
-                ),
-              }),
-            );
-          }
-          break;
-      }
+      const e_ = new errors.ErrorQUICClientInternal(
+        'Failed to send data on the QUICSocket',
+        {
+          data: evt.detail,
+          cause: e,
+        },
+      );
+      this.dispatchEvent(new events.EventQUICClientError({ detail: e_ }));
     }
   };
 
diff --git a/src/QUICConnection.ts b/src/QUICConnection.ts
@@ -62,11 +62,6 @@ class QUICConnection {
    */
   public readonly streamMap: Map<StreamId, QUICStream> = new Map();
 
-  /**
-   * Unique id used to identify events intended for this connection.
-   */
-  public readonly sendId: string;
-
   protected logger: Logger;
   protected socket: QUICSocket;
   protected config: QUICConfig;
@@ -327,7 +322,6 @@ class QUICConnection {
         logger?: Logger;
       }) {
     this.logger = logger ?? new Logger(`${this.constructor.name} ${scid}`);
-    this.sendId = scid.toString();
     if (
       config.keepAliveIntervalTime != null &&
       config.maxIdleTimeout !== 0 &&
@@ -882,7 +876,6 @@ class QUICConnection {
       this.dispatchEvent(
         new events.EventQUICConnectionSend({
           detail: {
-            id: this.sendId,
             msg: sendBuffer.subarray(0, sendLength),
             port: sendInfo.to.port,
             address: sendInfo.to.host,
diff --git a/src/QUICServer.ts b/src/QUICServer.ts
@@ -62,10 +62,6 @@ class QUICServer {
   protected _closed: boolean = false;
   protected _closedP: Promise<void>;
   protected resolveClosedP: () => void;
-  /**
-   * Flag used to make sure network fail warnings are only logged once per failure
-   */
-  protected networkWarned: boolean = false;
 
   /**
    * Handles `EventQUICServerError`.
@@ -199,49 +195,15 @@ class QUICServer {
         evt.detail.port,
         evt.detail.address,
       );
-      this.networkWarned = false;
     } catch (e) {
-      switch (e.code) {
-        case 'EINVAL':
-          {
-            this.dispatchEvent(
-              new events.EventQUICClientErrorSend(
-                `${events.EventQUICClientErrorSend.name}-${evt.detail.id}`,
-                {
-                  detail: e,
-                },
-              ),
-            );
-          }
-          break;
-        case 'ENETUNREACH':
-          {
-            // We consider this branch a temp failure.
-            // For these error codes we rely on the connection's timeout to handle.
-            if (!this.networkWarned) {
-              this.logger.warn(
-                `server send failed with 'ENETUNREACH', likely due to network failure`,
-              );
-              this.networkWarned = true;
-            }
-          }
-          break;
-        default:
-          {
-            this.dispatchEvent(
-              new events.EventQUICServerError({
-                detail: new errors.ErrorQUICServerInternal(
-                  'Failed to send data on the QUICSocket',
-                  {
-                    data: evt.detail,
-                    cause: e,
-                  },
-                ),
-              }),
-            );
-          }
-          break;
-      }
+      const e_ = new errors.ErrorQUICServerInternal(
+        'Failed to send data on the QUICSocket',
+        {
+          data: evt.detail,
+          cause: e,
+        },
+      );
+      this.dispatchEvent(new events.EventQUICServerError({ detail: e_ }));
     }
   };
 
diff --git a/src/errors.ts b/src/errors.ts
@@ -59,11 +59,6 @@ class ErrorQUICClientSocketNotRunning<T> extends ErrorQUICClient<T> {
     'QUIC Client cannot be created with an unstarted shared QUIC socket';
 }
 
-class ErrorQUICClientInvalidArgument<T> extends ErrorQUICClient<T> {
-  static description =
-    'QUIC Client had a failure relating to an invalid argument';
-}
-
 class ErrorQUICClientInvalidHost<T> extends ErrorQUICClient<T> {
   static description = 'QUIC Client cannot be created with the specified host';
 }
@@ -298,7 +293,6 @@ export {
   ErrorQUICClientDestroyed,
   ErrorQUICClientCreateTimeout,
   ErrorQUICClientSocketNotRunning,
-  ErrorQUICClientInvalidArgument,
   ErrorQUICClientInvalidHost,
   ErrorQUICClientInternal,
   ErrorQUICServer,
diff --git a/src/events.ts b/src/events.ts
@@ -62,8 +62,6 @@ class EventQUICClientError extends EventQUICClient<
   | ErrorQUICConnectionInternal<unknown>
 > {}
 
-class EventQUICClientErrorSend extends EventQUICSocket<Error> {}
-
 class EventQUICClientClose extends EventQUICClient<
   | ErrorQUICClientSocketNotRunning<unknown>
   | ErrorQUICConnectionLocal<unknown>
@@ -128,7 +126,6 @@ class EventQUICConnectionClose extends EventQUICConnection<
 class EventQUICConnectionStream extends EventQUICConnection<QUICStream> {}
 
 class EventQUICConnectionSend extends EventQUICConnection<{
-  id: string;
   msg: Uint8Array;
   port: number;
   address: string;
@@ -196,7 +193,6 @@ export {
   EventQUICClientDestroy,
   EventQUICClientDestroyed,
   EventQUICClientError,
-  EventQUICClientErrorSend,
   EventQUICClientClose,
   EventQUICServer,
   EventQUICServerStart,
diff --git a/tests/QUICStream.test.ts b/tests/QUICStream.test.ts
