Update the error test cases

ChainSafe · Aug 1, 2023 · 9439173 · 9439173
1 parent 5364794
commit 9439173
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 29 deletions.
diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts
@@ -129,10 +129,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
       this.updateEngineState(ExecutionEngineState.ONLINE);
       return res;
     } catch (err) {
-      const newState = getExecutionEngineState({payloadError: err, oldState: this.state});
-      if (newState !== undefined) {
-        this.updateEngineState(newState);
-      }
+      this.updateEngineState(getExecutionEngineState({payloadError: err, oldState: this.state}));
 
       // TODO: This is case where we can't determine the nature of error.
       // We should throw only for such cases not for the which we knew the status

diff --git a/packages/beacon-node/src/execution/engine/utils.ts b/packages/beacon-node/src/execution/engine/utils.ts
@@ -51,17 +51,16 @@ function getExecutionEngineStateForPayloadStatus(payloadStatus: ExecutePayloadSt
     case ExecutePayloadStatus.UNAVAILABLE:
       return ExecutionEngineState.OFFLINE;
 
-    // In case we can't determine the state, we assume it's online
-    // This assumption is better than considering offline, because the offline state may trigger some notifications
     default:
+      // In case we can't determine the state, we assume it stays in old state
       return ExecutionEngineState.ONLINE;
   }
 }
 
 function getExecutionEngineStateForPayloadError(
   payloadError: unknown,
   oldState: ExecutionEngineState
-): ExecutionEngineState | undefined {
+): ExecutionEngineState {
   if (isErrorAborted(payloadError) || isQueueErrorAborted(payloadError)) {
     return oldState;
   }
@@ -79,35 +78,30 @@ function getExecutionEngineStateForPayloadError(
     return ExecutionEngineState.AUTH_FAILED;
   }
 
-  return undefined;
+  return oldState;
 }
 
-export function getExecutionEngineState<
-  S extends ExecutePayloadStatus | undefined,
-  E extends unknown | undefined,
-  R = S extends undefined ? ExecutionEngineState | undefined : ExecutionEngineState,
->({
+export function getExecutionEngineState<S extends ExecutePayloadStatus | undefined, E extends unknown | undefined>({
   payloadError,
   payloadStatus,
   oldState,
 }:
   | {payloadStatus: S; payloadError?: never; oldState: ExecutionEngineState}
-  | {payloadStatus?: never; payloadError: E; oldState: ExecutionEngineState}): R {
+  | {payloadStatus?: never; payloadError: E; oldState: ExecutionEngineState}): ExecutionEngineState {
   const newState =
     payloadStatus === undefined
       ? getExecutionEngineStateForPayloadError(payloadError, oldState)
       : getExecutionEngineStateForPayloadStatus(payloadStatus);
 
-  // The `payloadError` is something based on which we can't determine the state of execution engine
-  if (newState === undefined) return undefined as R;
+  if (newState === oldState) return oldState;
 
   // The ONLINE is initial state and can reached from offline or auth failed error
   if (
     newState === ExecutionEngineState.ONLINE &&
     !(oldState === ExecutionEngineState.OFFLINE || oldState === ExecutionEngineState.AUTH_FAILED)
   ) {
-    return oldState as R;
+    return oldState;
   }
 
-  return newState as R;
+  return newState;
 }
diff --git a/packages/beacon-node/test/unit/execution/engine/utils.test.ts b/packages/beacon-node/test/unit/execution/engine/utils.test.ts
@@ -10,7 +10,7 @@ import {
 import {QueueError, QueueErrorCode} from "../../../../src/util/queue/errors.js";
 import {ErrorJsonRpcResponse, HttpRpcError} from "../../../../src/eth1/provider/jsonRpcHttpClient.js";
 
-describe("execution/engine/utils", () => {
+describe("execution / engine / utils", () => {
   describe("getExecutionEngineState", () => {
     const testCasesPayload: Record<
       ExecutePayloadStatus,
@@ -81,11 +81,7 @@ describe("execution/engine/utils", () => {
       ],
     };
 
-    type ErrorTestCase = [
-      string,
-      Error,
-      [oldState: ExecutionEngineState, newState: ExecutionEngineState | undefined][],
-    ];
+    type ErrorTestCase = [string, Error, [oldState: ExecutionEngineState, newState: ExecutionEngineState][]];
     const testCasesError: ErrorTestCase[] = [
       [
         "abort error",
@@ -167,11 +163,11 @@ describe("execution/engine/utils", () => {
         "unknown error",
         new Error("unknown error"),
         [
-          [ExecutionEngineState.ONLINE, undefined],
-          [ExecutionEngineState.AUTH_FAILED, undefined],
-          [ExecutionEngineState.OFFLINE, undefined],
-          [ExecutionEngineState.SYNCED, undefined],
-          [ExecutionEngineState.SYNCING, undefined],
+          [ExecutionEngineState.ONLINE, ExecutionEngineState.ONLINE],
+          [ExecutionEngineState.AUTH_FAILED, ExecutionEngineState.AUTH_FAILED],
+          [ExecutionEngineState.OFFLINE, ExecutionEngineState.OFFLINE],
+          [ExecutionEngineState.SYNCED, ExecutionEngineState.SYNCED],
+          [ExecutionEngineState.SYNCING, ExecutionEngineState.SYNCING],
         ],
       ],
     ];