Go SDK: Keep audit interceptors non-blocking on errors (#36)
<!-- Provide a brief summary of your changes --> Ensure Go audit-mode interceptors remain non-blocking when validator or mutator invocation returns an error. ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> SEP says audit-mode interceptors MUST NOT block execution regardless of results. The Go SDK already avoided blocking on audit-mode validation findings, but validator/mutator handler or RPC errors could still abort unless `FailOpen` was set. This change makes audit mode consistently non-blocking for invocation errors as well, matching the existing C# orchestrator behavior where `isAudit || failOpen` allows execution to continue. ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> Added focused Go chain tests covering audit-mode validator and mutator errors. ## Breaking Changes <!-- Will users need to update their code or configurations? --> None. This only relaxes audit-mode error handling to match SEP semantics. ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions --> C# already treats audit-mode invocation failures as non-blocking. This aligns the Go chain behavior with that implementation and SEP audit-mode semantics. Signed-off-by: Ukjae Jeong <jeongukjae@gmail.com>
This commit is contained in:
parent
28ada74189
commit
1a3e5ef459
2 changed files with 86 additions and 2 deletions
|
|
@ -303,7 +303,7 @@ func (c *Chain) recordValidation(entry ChainEntry, result invokeOutcome, cr *Exe
|
|||
Type: interceptors.TypeValidation,
|
||||
Phase: cr.Phase,
|
||||
})
|
||||
if !entry.Interceptor.FailOpen {
|
||||
if result.mode != interceptors.ModeAudit && !entry.Interceptor.FailOpen {
|
||||
cr.AbortedAt = append(cr.AbortedAt, AbortInfo{
|
||||
Interceptor: entry.Interceptor.Name,
|
||||
Reason: result.err.Error(),
|
||||
|
|
@ -383,7 +383,7 @@ func (c *Chain) runMutators(
|
|||
Type: interceptors.TypeMutation,
|
||||
Phase: params.Phase,
|
||||
})
|
||||
if !m.Interceptor.FailOpen {
|
||||
if result.mode != interceptors.ModeAudit && !m.Interceptor.FailOpen {
|
||||
cr.AbortedAt = append(cr.AbortedAt, AbortInfo{
|
||||
Interceptor: m.Interceptor.Name,
|
||||
Reason: result.err.Error(),
|
||||
|
|
|
|||
|
|
@ -407,3 +407,87 @@ func TestChain_FailOpenRecordsExecutionResult(t *testing.T) {
|
|||
require.Len(t, cr.Results, 2)
|
||||
})
|
||||
}
|
||||
|
||||
func TestChain_AuditModeErrorsDoNotAbort(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("audit validator error is recorded without aborting", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
auditValidator := &interceptors.Validator{
|
||||
Metadata: interceptors.Metadata{
|
||||
Name: "audit-validator",
|
||||
Hooks: []interceptors.Hook{{
|
||||
Events: []string{"test/event"},
|
||||
Phase: interceptors.PhaseRequest,
|
||||
}},
|
||||
Mode: interceptors.ModeAudit,
|
||||
},
|
||||
Handler: func(_ context.Context, _ *interceptors.Invocation) (*interceptors.ValidationResult, error) {
|
||||
return nil, fmt.Errorf("audit sink failed")
|
||||
},
|
||||
}
|
||||
|
||||
ch := setupChainWithInterceptors(t, auditValidator)
|
||||
|
||||
payload, _ := json.Marshal(map[string]any{"value": "hello"})
|
||||
cr, err := ch.Execute(context.Background(), &chain.ExecutionParams{
|
||||
Event: "test/event",
|
||||
Phase: interceptors.PhaseRequest,
|
||||
Payload: payload,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, chain.ChainSuccess, cr.Status)
|
||||
assert.Empty(t, cr.AbortedAt)
|
||||
require.Len(t, cr.Results, 1)
|
||||
assert.Equal(t, "audit-validator", cr.Results[0].Interceptor)
|
||||
})
|
||||
|
||||
t.Run("audit mutator error is recorded without aborting", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
auditMutator := &interceptors.Mutator{
|
||||
Metadata: interceptors.Metadata{
|
||||
Name: "audit-mutator",
|
||||
Hooks: []interceptors.Hook{{
|
||||
Events: []string{"test/event"},
|
||||
Phase: interceptors.PhaseResponse,
|
||||
}},
|
||||
Mode: interceptors.ModeAudit,
|
||||
PriorityHint: interceptors.NewPriority(10),
|
||||
},
|
||||
Handler: func(_ context.Context, _ *interceptors.Invocation) (*interceptors.MutationResult, error) {
|
||||
return nil, fmt.Errorf("shadow mutation failed")
|
||||
},
|
||||
}
|
||||
passingMutator := &interceptors.Mutator{
|
||||
Metadata: interceptors.Metadata{
|
||||
Name: "passing-mutator",
|
||||
Hooks: []interceptors.Hook{{
|
||||
Events: []string{"test/event"},
|
||||
Phase: interceptors.PhaseResponse,
|
||||
}},
|
||||
Mode: interceptors.ModeEnforce,
|
||||
PriorityHint: interceptors.NewPriority(20),
|
||||
},
|
||||
Handler: func(_ context.Context, _ *interceptors.Invocation) (*interceptors.MutationResult, error) {
|
||||
modified, _ := json.Marshal(map[string]any{"value": "mutated"})
|
||||
return &interceptors.MutationResult{Modified: true, Payload: modified}, nil
|
||||
},
|
||||
}
|
||||
|
||||
ch := setupChainWithInterceptors(t, auditMutator, passingMutator)
|
||||
|
||||
payload, _ := json.Marshal(map[string]any{"value": "hello"})
|
||||
cr, err := ch.Execute(context.Background(), &chain.ExecutionParams{
|
||||
Event: "test/event",
|
||||
Phase: interceptors.PhaseResponse,
|
||||
Payload: payload,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, chain.ChainSuccess, cr.Status)
|
||||
assert.Empty(t, cr.AbortedAt)
|
||||
require.Len(t, cr.Results, 2)
|
||||
require.NotNil(t, cr.FinalPayload)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue