From 45e3c9fa27ea28d9177cf1652019c30233d954db Mon Sep 17 00:00:00 2001 From: shenjiayun Date: Mon, 26 Oct 2026 11:56:25 +0800 Subject: [PATCH 1/2] [fix][evaluation] classify missing target callback context as not found --- .../application/eval_openapi_app.go | 7 +- .../application/eval_openapi_app_test.go | 126 ++++++++++++++++++ 2 files changed, 130 insertions(+), 3 deletions(-) diff --git a/backend/modules/evaluation/application/eval_openapi_app.go b/backend/modules/evaluation/application/eval_openapi_app.go index 653d5f8d4..6616403e6 100644 --- a/backend/modules/evaluation/application/eval_openapi_app.go +++ b/backend/modules/evaluation/application/eval_openapi_app.go @@ -13,6 +13,7 @@ import ( "github.com/bytedance/gg/gmap" "github.com/bytedance/gg/gptr" usersession "github.com/coze-dev/coze-loop/backend/infra/middleware/session" + "github.com/coze-dev/coze-loop/backend/infra/redis" "github.com/coze-dev/coze-loop/backend/kitex_gen/base" "github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/evaluation" domaincommon "github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/evaluation/domain/common" @@ -1303,13 +1304,13 @@ func (e *EvalOpenAPIApplication) ReportEvalTargetInvokeResult_(ctx context.Conte logs.CtxInfo(ctx, "ReportEvalTargetInvokeResult receive req: %v", json.Jsonify(req)) actx, err := e.asyncRepo.GetEvalAsyncCtx(ctx, strconv.FormatInt(req.GetInvokeID(), 10)) - if err != nil { + if err != nil && !redis.IsNilError(err) { return nil, err } - if actx == nil { + if err != nil || actx == nil { logs.CtxWarn(ctx, "report target record, actx missing, invoke_id: %v, space_id: %v", req.GetInvokeID(), req.GetWorkspaceID()) - return nil, errorx.New("eval async context not found, invoke_id: %v", req.GetInvokeID()) + return nil, errorx.NewByCode(errno.ResourceNotFoundCode, errorx.WithExtraMsg(fmt.Sprintf("eval async context not found, invoke_id: %v", req.GetInvokeID()))) } // 调试场景（actx.Event == nil）：无论成功失败都 best-effort 销毁沙箱执行。 diff --git a/backend/modules/evaluation/application/eval_openapi_app_test.go b/backend/modules/evaluation/application/eval_openapi_app_test.go index ac4ea1383..30e3ea9fd 100755 --- a/backend/modules/evaluation/application/eval_openapi_app_test.go +++ b/backend/modules/evaluation/application/eval_openapi_app_test.go @@ -6,11 +6,17 @@ package application import ( "context" "errors" + "fmt" + "net" "strconv" + "syscall" "testing" "time" + "github.com/alicebob/miniredis/v2" "github.com/bytedance/gg/gptr" + "github.com/cloudwego/kitex/pkg/kerrors" + goredis "github.com/redis/go-redis/v9" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -40,6 +46,7 @@ import ( repomocks "github.com/coze-dev/coze-loop/backend/modules/evaluation/domain/repo/mocks" "github.com/coze-dev/coze-loop/backend/modules/evaluation/domain/service" servicemocks "github.com/coze-dev/coze-loop/backend/modules/evaluation/domain/service/mocks" + experimentrepo "github.com/coze-dev/coze-loop/backend/modules/evaluation/infra/repo/experiment" experimentredisdao "github.com/coze-dev/coze-loop/backend/modules/evaluation/infra/repo/experiment/redis/dao" "github.com/coze-dev/coze-loop/backend/modules/evaluation/pkg/errno" "github.com/coze-dev/coze-loop/backend/pkg/errorx" @@ -2199,6 +2206,102 @@ func TestEvalOpenAPIApplication_UpdateEvaluationSetSchemaOApi(t *testing.T) { } } +func TestEvalOpenAPIApplication_ReportEvalTargetInvokeResult_ContextErrors(t *testing.T) { + t.Parallel() + + connectionErr := &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED} + tests := []struct { + name string + actx *entity.EvalAsyncCtx + repoErr error + wantMiss bool + }{ + {name: "redis nil", repoErr: goredis.Nil, wantMiss: true}, + {name: "joined redis nil", repoErr: errors.Join(goredis.Nil, errors.New("redis: nil")), wantMiss: true}, + {name: "wrapped redis nil", repoErr: fmt.Errorf("load async context: %w", goredis.Nil), wantMiss: true}, + {name: "stack wrapped redis nil", repoErr: errorx.Wrapf(goredis.Nil, "redis get fail"), wantMiss: true}, + {name: "nil context", wantMiss: true}, + {name: "redis timeout", repoErr: context.DeadlineExceeded}, + {name: "redis connection refused", repoErr: connectionErr}, + {name: "wrapped redis connection refused", repoErr: errorx.Wrapf(connectionErr, "redis get fail")}, + {name: "error takes precedence over context", actx: &entity.EvalAsyncCtx{}, repoErr: context.DeadlineExceeded}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + asyncRepo := repomocks.NewMockIEvalAsyncRepo(ctrl) + req := newSuccessInvokeResultReq(11, 101) + asyncRepo.EXPECT().GetEvalAsyncCtx(gomock.Any(), "101").Return(tt.actx, tt.repoErr) + app := &EvalOpenAPIApplication{ + asyncRepo: asyncRepo, + targetSvc: servicemocks.NewMockIEvalTargetService(ctrl), + publisher: eventmocks.NewMockExptEventPublisher(ctrl), + } + + resp, err := app.ReportEvalTargetInvokeResult_(context.Background(), req) + require.Error(t, err) + assert.Nil(t, resp) + statusErr, isStatusErr := errorx.FromStatusError(err) + if tt.wantMiss { + require.True(t, isStatusErr, "missing context must return a business status error") + assert.Equal(t, int32(errno.ResourceNotFoundCode), statusErr.Code()) + bizErr, isBizErr := kerrors.FromBizStatusError(err) + require.True(t, isBizErr) + assert.Equal(t, "0", bizErr.BizExtra()["biz_err_affect_stability"]) + return + } + assert.True(t, err == tt.repoErr, "Redis failures must be returned unchanged") + assert.False(t, isStatusErr, "Redis failures must remain system errors") + }) + } +} + +func TestEvalOpenAPIApplication_ReportEvalTargetInvokeResult_RedisErrors(t *testing.T) { + t.Parallel() + + for _, name := range []string{"missing key", "connection refused", "deadline exceeded"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + server := miniredis.RunT(t) + client, err := infraredis.NewClient(&goredis.Options{ + Addr: server.Addr(), MaxRetries: -1, DialTimeout: 100 * time.Millisecond, + }) + require.NoError(t, err) + raw, ok := infraredis.Unwrap(client) + require.True(t, ok) + t.Cleanup(func() { _ = raw.Close() }) + ctx := context.Background() + if name == "connection refused" { + server.Close() + } else if name == "deadline exceeded" { + var cancel context.CancelFunc + ctx, cancel = context.WithDeadline(ctx, time.Now().Add(-time.Second)) + defer cancel() + } + app := &EvalOpenAPIApplication{ + asyncRepo: experimentrepo.NewEvalAsyncRepo(experimentredisdao.NewEvalAsyncDAO(client, nil)), + } + + resp, err := app.ReportEvalTargetInvokeResult_(ctx, newSuccessInvokeResultReq(11, 101)) + require.Error(t, err) + assert.Nil(t, resp) + statusErr, isStatusErr := errorx.FromStatusError(err) + switch name { + case "missing key": + require.True(t, isStatusErr) + assert.Equal(t, int32(errno.ResourceNotFoundCode), statusErr.Code()) + case "connection refused": + assert.ErrorIs(t, err, syscall.ECONNREFUSED) + assert.False(t, isStatusErr) + case "deadline exceeded": + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.False(t, isStatusErr) + } + }) + } +} + func TestEvalOpenAPIApplication_ReportEvalTargetInvokeResult(t *testing.T) { t.Parallel() @@ -2292,6 +2395,29 @@ func TestEvalOpenAPIApplication_ReportEvalTargetInvokeResult(t *testing.T) { }, wantErr: false, }, + { + name: "success with event", + req: successReq, + setup: func(t *testing.T, asyncRepo *repomocks.MockIEvalAsyncRepo, targetSvc *servicemocks.MockIEvalTargetService, publisher *eventmocks.MockExptEventPublisher, configer *configermocks.MockIConfiger) { + event := &entity.ExptItemEvalEvent{SpaceID: 44, ExptID: 1, ExptRunID: 2, EvalSetItemID: 3} + actx := &entity.EvalAsyncCtx{AsyncUnixMS: time.Now().Add(-100 * time.Millisecond).UnixMilli(), Event: event} + asyncRepo.EXPECT().GetEvalAsyncCtx(gomock.Any(), "404").Return(actx, nil) + report := targetSvc.EXPECT().ReportInvokeRecords(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, param *entity.ReportTargetRecordParam) error { + assert.Equal(t, int64(44), param.SpaceID) + assert.Equal(t, int64(404), param.RecordID) + assert.Equal(t, entity.EvalTargetRunStatusSuccess, param.Status) + return nil + }) + configer.EXPECT().GetTargetTrajectoryConf(gomock.Any()).Return(&entity.TargetTrajectoryConf{}) + publisher.EXPECT().PublishExptRecordEvalEvent(gomock.Any(), event, gomock.Any(), gomock.Any()).After(report).DoAndReturn(func(_ context.Context, evt *entity.ExptItemEvalEvent, duration *time.Duration, option func(*entity.ExptItemEvalEvent)) error { + require.NotNil(t, duration) + assert.Equal(t, 50*time.Second, *duration) + option(evt) + assert.True(t, evt.AsyncReportTrigger) + return nil + }) + }, + }, { name: "success with event on failure status", req: failedReq, From c2a73c659a800f3a811e2d24b78b9b9eac568356 Mon Sep 17 00:00:00 2001 From: shenjiayun Date: Mon, 26 Oct 2026 12:07:16 +0800 Subject: [PATCH 2/2] [test][evaluation] use a tagged switch in callback error cases --- .../modules/evaluation/application/eval_openapi_app_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/modules/evaluation/application/eval_openapi_app_test.go b/backend/modules/evaluation/application/eval_openapi_app_test.go index 30e3ea9fd..ccb6b4873 100755 --- a/backend/modules/evaluation/application/eval_openapi_app_test.go +++ b/backend/modules/evaluation/application/eval_openapi_app_test.go @@ -2272,9 +2272,10 @@ func TestEvalOpenAPIApplication_ReportEvalTargetInvokeResult_RedisErrors(t *test require.True(t, ok) t.Cleanup(func() { _ = raw.Close() }) ctx := context.Background() - if name == "connection refused" { + switch name { + case "connection refused": server.Close() - } else if name == "deadline exceeded" { + case "deadline exceeded": var cancel context.CancelFunc ctx, cancel = context.WithDeadline(ctx, time.Now().Add(-time.Second)) defer cancel()