Merge pull request #5036 from onflow/petera/5030-handle-script-cancel…

…ed-errors

[Access] Handle script canceled and timeout errors

engine/access/rpc/backend/backend_scripts.go

@@ -125,9 +125,11 @@ func (b *backendScripts) executeScript(
 
 	case ScriptExecutionModeFailover:
 		localResult, localDuration, localErr := b.executeScriptLocally(ctx, scriptRequest)
-		if localErr == nil || isInvalidArgumentError(localErr) {
+		if localErr == nil || isInvalidArgumentError(localErr) || status.Code(localErr) == codes.Canceled {
 			return localResult, localErr
 		}
+		// Note: scripts that timeout are retried on the execution nodes since ANs may have performance
+		// issues for some scripts.
 		execResult, execDuration, execErr := b.executeScriptOnAvailableExecutionNodes(ctx, scriptRequest)
 
 		resultComparer := newScriptResultComparison(b.log, b.metrics, scriptRequest)
@@ -185,11 +187,13 @@ func (b *backendScripts) executeScriptLocally(
 	if err != nil {
 		convertedErr := convertScriptExecutionError(err, r.height)
 
-		if status.Code(convertedErr) == codes.InvalidArgument {
+		switch status.Code(convertedErr) {
+		case codes.InvalidArgument, codes.Canceled, codes.DeadlineExceeded:
 			lg.Debug().Err(err).
 				Str("script", string(r.script)).
 				Msg("script failed to execute locally")
-		} else {
+
+		default:
 			lg.Error().Err(err).Msg("script execution failed")
 			b.metrics.ScriptExecutionErrorLocal()
 		}
@@ -332,8 +336,17 @@ func convertScriptExecutionError(err error, height uint64) error {
 			return rpc.ConvertError(err, "failed to execute script", codes.Internal)
 		}
 
-		// runtime errors
-		return status.Errorf(codes.InvalidArgument, "failed to execute script: %v", err)
+		switch coded.Code() {
+		case fvmerrors.ErrCodeScriptExecutionCancelledError:
+			return status.Errorf(codes.Canceled, "script execution canceled: %v", err)
+
+		case fvmerrors.ErrCodeScriptExecutionTimedOutError:
+			return status.Errorf(codes.DeadlineExceeded, "script execution timed out: %v", err)
+
+		default:
+			// runtime errors
+			return status.Errorf(codes.InvalidArgument, "failed to execute script: %v", err)
+		}
 	}
 
 	return convertIndexError(err, height, "failed to execute script")

engine/access/rpc/backend/backend_scripts_test.go

@@ -34,6 +34,8 @@ var (
 
 	cadenceErr    = fvmerrors.NewCodedError(fvmerrors.ErrCodeCadenceRunTimeError, "cadence error")
 	fvmFailureErr = fvmerrors.NewCodedError(fvmerrors.FailureCodeBlockFinderFailure, "fvm error")
+	ctxCancelErr  = fvmerrors.NewCodedError(fvmerrors.ErrCodeScriptExecutionCancelledError, "context canceled error")
+	timeoutErr    = fvmerrors.NewCodedError(fvmerrors.ErrCodeScriptExecutionTimedOutError, "timeout error")
 )
 
 // Create a suite similar to GetAccount that covers each of the modes
@@ -319,31 +321,49 @@ func (s *BackendScriptsSuite) TestExecuteScriptWithFailover_HappyPath() {
 	}
 }
 
-// TestExecuteScriptWithFailover_SkippedForInvalidArgument tests that failover is skipped for
-// FVM errors that result in InvalidArgument errors
-func (s *BackendScriptsSuite) TestExecuteScriptWithFailover_SkippedForInvalidArgument() {
+// TestExecuteScriptWithFailover_SkippedForCorrectCodes tests that failover is skipped for
+// FVM errors that result in InvalidArgument or Canceled errors
+func (s *BackendScriptsSuite) TestExecuteScriptWithFailover_SkippedForCorrectCodes() {
 	ctx := context.Background()
 
 	// configure local script executor to fail
 	scriptExecutor := execmock.NewScriptExecutor(s.T())
-	scriptExecutor.On("ExecuteAtBlockHeight", mock.Anything, s.failingScript, s.arguments, s.block.Header.Height).
-		Return(nil, cadenceErr)
 
 	backend := s.defaultBackend()
 	backend.scriptExecMode = ScriptExecutionModeFailover
 	backend.scriptExecutor = scriptExecutor
 
-	s.Run("ExecuteScriptAtLatestBlock", func() {
-		s.testExecuteScriptAtLatestBlock(ctx, backend, codes.InvalidArgument)
-	})
+	testCases := []struct {
+		err        error
+		statusCode codes.Code
+	}{
+		{
+			err:        cadenceErr,
+			statusCode: codes.InvalidArgument,
+		},
+		{
+			err:        ctxCancelErr,
+			statusCode: codes.Canceled,
+		},
+	}
 
-	s.Run("ExecuteScriptAtBlockID", func() {
-		s.testExecuteScriptAtBlockID(ctx, backend, codes.InvalidArgument)
-	})
+	for _, tt := range testCases {
+		scriptExecutor.On("ExecuteAtBlockHeight", mock.Anything, s.failingScript, s.arguments, s.block.Header.Height).
+			Return(nil, tt.err).
+			Times(3)
 
-	s.Run("ExecuteScriptAtBlockHeight", func() {
-		s.testExecuteScriptAtBlockHeight(ctx, backend, codes.InvalidArgument)
-	})
+		s.Run(fmt.Sprintf("ExecuteScriptAtLatestBlock - %s", tt.statusCode), func() {
+			s.testExecuteScriptAtLatestBlock(ctx, backend, tt.statusCode)
+		})
+
+		s.Run(fmt.Sprintf("ExecuteScriptAtBlockID - %s", tt.statusCode), func() {
+			s.testExecuteScriptAtBlockID(ctx, backend, tt.statusCode)
+		})
+
+		s.Run(fmt.Sprintf("ExecuteScriptAtBlockHeight - %s", tt.statusCode), func() {
+			s.testExecuteScriptAtBlockHeight(ctx, backend, tt.statusCode)
+		})
+	}
 }
 
 // TestExecuteScriptWithFailover_ReturnsENErrors tests that when an error is returned from the execution

fvm/errors/codes.go

@@ -78,8 +78,8 @@ const (
 	ErrCodeComputationLimitExceededError             ErrorCode = 1110
 	ErrCodeMemoryLimitExceededError                  ErrorCode = 1111
 	ErrCodeCouldNotDecodeExecutionParameterFromState ErrorCode = 1112
-	ErrCodeScriptExecutionCancelledError             ErrorCode = 1114
 	ErrCodeScriptExecutionTimedOutError              ErrorCode = 1113
+	ErrCodeScriptExecutionCancelledError             ErrorCode = 1114
 	ErrCodeEventEncodingError                        ErrorCode = 1115
 	ErrCodeInvalidInternalStateAccessError           ErrorCode = 1116
 	// 1117 was never deployed and is free to use