From 48b2212ecda41cd987822709185f3b14c22a181e Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Fri, 17 Nov 2023 10:37:59 -0800 Subject: [PATCH 1/2] [Access] Handle script canceled and timeout errors --- engine/access/rpc/backend/backend_scripts.go | 23 +++++++-- .../rpc/backend/backend_scripts_test.go | 48 +++++++++++++------ fvm/errors/codes.go | 2 +- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/engine/access/rpc/backend/backend_scripts.go b/engine/access/rpc/backend/backend_scripts.go index 3d8591817d9..05398ccf9c2 100644 --- a/engine/access/rpc/backend/backend_scripts.go +++ b/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") diff --git a/engine/access/rpc/backend/backend_scripts_test.go b/engine/access/rpc/backend/backend_scripts_test.go index 951adc9b50c..7df195b0fa3 100644 --- a/engine/access/rpc/backend/backend_scripts_test.go +++ b/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, "ctx 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 diff --git a/fvm/errors/codes.go b/fvm/errors/codes.go index 3308b47fdd9..cdbc734bd3d 100644 --- a/fvm/errors/codes.go +++ b/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 From b28e8676127e6373d435aec1933341706b0b786d Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Mon, 20 Nov 2023 09:52:27 -0800 Subject: [PATCH 2/2] Update engine/access/rpc/backend/backend_scripts_test.go Co-authored-by: Gregor G. <75445744+sideninja@users.noreply.github.com> --- engine/access/rpc/backend/backend_scripts_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/access/rpc/backend/backend_scripts_test.go b/engine/access/rpc/backend/backend_scripts_test.go index 7df195b0fa3..bb734cab657 100644 --- a/engine/access/rpc/backend/backend_scripts_test.go +++ b/engine/access/rpc/backend/backend_scripts_test.go @@ -34,7 +34,7 @@ var ( cadenceErr = fvmerrors.NewCodedError(fvmerrors.ErrCodeCadenceRunTimeError, "cadence error") fvmFailureErr = fvmerrors.NewCodedError(fvmerrors.FailureCodeBlockFinderFailure, "fvm error") - ctxCancelErr = fvmerrors.NewCodedError(fvmerrors.ErrCodeScriptExecutionCancelledError, "ctx canceled error") + ctxCancelErr = fvmerrors.NewCodedError(fvmerrors.ErrCodeScriptExecutionCancelledError, "context canceled error") timeoutErr = fvmerrors.NewCodedError(fvmerrors.ErrCodeScriptExecutionTimedOutError, "timeout error") )