mirror of
https://github.com/zap-proto/zip.git
synced 2026-10-19 15:38:57 +00:00
OnResult: the counterpart to Authorize, told what happened
Authorize is ASKED before a handler and may refuse. Nothing is TOLD after one, which leaves a gap that no middleware can close: everything a middleware records is recorded about a REQUEST, and an in-process invoke is not one. An operation reached that way runs with no trace of having run — no trail entry, no metric, no span — and it is the door an operator on a pod uses. So the hook sits at the same seam, which is the one place every projection funnels through, and is told about all of them. A refusal is told about as surely as a success, because Deny returns through the same error it reads. It is told the operation and the outcome and NOTHING else. Not the output: a hook that could see the value would be a way for a response body to reach whatever the hook writes to, and every recorder in this estate promises it never reads bodies. Not the input either — Authorize already sees that, before the handler, where a decision belongs. Named returns made that possible and cost one bug on the way: binding the handler's result to the `any` return wraps a nil *Out in a non-nil interface, so a void op stopped answering 204. opstatus_test.go caught it, the result stays typed, and the comment says why. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
@@ -159,6 +159,37 @@ type Authorizer func(ctx context.Context, op Op, in any) (Decision, error)
|
||||
// honest way to serve it under both. See [adopt].
|
||||
func (a *App) Authorize(fn Authorizer) { a.authorizer = fn }
|
||||
|
||||
// OnResult installs fn as the rule for what to do with the OUTCOME of every
|
||||
// typed op this app serves. It is [App.Authorize]'s counterpart: Authorize is
|
||||
// ASKED before the handler and may refuse, OnResult is TOLD after it and may
|
||||
// not. Both run at the one invoke seam, so both cover every projection — the
|
||||
// REST route, tools/call at /mcp, the by-name call plane, the graph, and the
|
||||
// in-process CLI, which is the only one no transport middleware can wrap.
|
||||
//
|
||||
// That last one is why this exists. Anything a middleware records — a trail, a
|
||||
// metric, a span — is recorded about a REQUEST, and an in-process invoke is not
|
||||
// one. A recorder installed here is told about it under the same operation name
|
||||
// as every other door.
|
||||
//
|
||||
// fn is told the operation and the error, and nothing else. The output is
|
||||
// deliberately absent: a hook that could read it would be a way for a response
|
||||
// body to reach whatever the hook writes to. Call once while mounting, before
|
||||
// Listen — the field is read on serve goroutines. A nil fn declares no hook.
|
||||
func (a *App) OnResult(fn func(ctx context.Context, op Op, err error)) { a.onResult = fn }
|
||||
|
||||
// result is the [App.OnResult] hook in force over this App's ops: its own if it
|
||||
// declared one, else the one it was composed under — the same resolution
|
||||
// [App.rule] uses, for the same reason.
|
||||
func (a *App) result() func(context.Context, Op, error) {
|
||||
if a.onResult != nil {
|
||||
return a.onResult
|
||||
}
|
||||
if over := a.over.Load(); over != nil {
|
||||
return over.onResult
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// rule is the Authorizer in force over this App's ops: its own if it declared
|
||||
// one, else the one it was composed under.
|
||||
//
|
||||
|
||||
+176
@@ -0,0 +1,176 @@
|
||||
package zip_test
|
||||
|
||||
// OnResult is told about every way an op can be reached, including the one no
|
||||
// middleware can wrap.
|
||||
//
|
||||
// Everything a transport middleware records — a trail, a metric, a span — is
|
||||
// recorded about a REQUEST. An in-process invoke is not one: it has no request,
|
||||
// so nothing wraps it, and an operation reached that way leaves no trace of
|
||||
// having run. That is the gap this hook exists for, and it is the row below that
|
||||
// no other mechanism in the package can produce.
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/zap-proto/zip"
|
||||
)
|
||||
|
||||
type resIn struct {
|
||||
Fail bool `json:"fail"`
|
||||
}
|
||||
|
||||
type resOut struct {
|
||||
OK bool `json:"ok"`
|
||||
}
|
||||
|
||||
// told is one thing the hook was told.
|
||||
type told struct {
|
||||
op string
|
||||
err error
|
||||
}
|
||||
|
||||
func resultApp(t *testing.T, seen *[]told) *zip.App {
|
||||
t.Helper()
|
||||
app := zip.New(zip.Config{AppName: "restest", DisableStartupMessage: true})
|
||||
zip.Post(app, "/v1/things", func(_ context.Context, in *resIn) (*resOut, error) {
|
||||
if in.Fail {
|
||||
return nil, errors.New("the handler said no")
|
||||
}
|
||||
return &resOut{OK: true}, nil
|
||||
}, zip.WithOperationID("createThing"))
|
||||
app.OnResult(func(_ context.Context, op zip.Op, err error) {
|
||||
*seen = append(*seen, told{op: op.OperationID, err: err})
|
||||
})
|
||||
if err := app.Build(); err != nil {
|
||||
t.Fatalf("Build: %v", err)
|
||||
}
|
||||
return app
|
||||
}
|
||||
|
||||
func TestOnResult_IsToldAboutEveryProjection(t *testing.T) {
|
||||
var seen []told
|
||||
app := resultApp(t, &seen)
|
||||
|
||||
post := func(path string, body any) {
|
||||
t.Helper()
|
||||
b, _ := json.Marshal(body)
|
||||
req := httptest.NewRequest("POST", path, bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
resp, err := app.Fiber().Test(req)
|
||||
if err != nil {
|
||||
t.Fatalf("%s: %v", path, err)
|
||||
}
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
|
||||
post("/v1/things", map[string]any{"fail": false})
|
||||
post("/mcp", map[string]any{
|
||||
"jsonrpc": "2.0", "id": 1, "method": "tools/call",
|
||||
"params": map[string]any{"name": "createThing", "arguments": map[string]any{"fail": false}},
|
||||
})
|
||||
// The in-process seam: no request, no middleware, and until this hook no
|
||||
// record that it happened at all.
|
||||
if _, err := zip.Here[resIn, resOut](context.Background(), app, "createThing", &resIn{}); err != nil {
|
||||
t.Fatalf("in-process invoke: %v", err)
|
||||
}
|
||||
|
||||
if len(seen) != 3 {
|
||||
t.Fatalf("the hook was told %d times, want 3 (REST, MCP, in-process): %+v", len(seen), seen)
|
||||
}
|
||||
for i, s := range seen {
|
||||
if s.op != "createThing" {
|
||||
t.Errorf("told %d names operation %q, want createThing", i, s.op)
|
||||
}
|
||||
if s.err != nil {
|
||||
t.Errorf("told %d carries err %v, want nil", i, s.err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A FAILURE IS AN OUTCOME. A hook told only about successes would record a
|
||||
// surface that never fails, which is the opposite of what a trail is for.
|
||||
func TestOnResult_IsToldWhenTheHandlerFails(t *testing.T) {
|
||||
var seen []told
|
||||
app := resultApp(t, &seen)
|
||||
|
||||
b, _ := json.Marshal(map[string]any{"fail": true})
|
||||
req := httptest.NewRequest("POST", "/v1/things", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
resp, err := app.Fiber().Test(req)
|
||||
if err != nil {
|
||||
t.Fatalf("REST: %v", err)
|
||||
}
|
||||
_ = resp.Body.Close()
|
||||
|
||||
if len(seen) != 1 {
|
||||
t.Fatalf("the hook was told %d times, want 1", len(seen))
|
||||
}
|
||||
if seen[0].err == nil {
|
||||
t.Error("the hook was told nil for a handler that returned an error")
|
||||
}
|
||||
}
|
||||
|
||||
// A REFUSAL IS AN OUTCOME TOO, and the one a trail most needs: a caller that was
|
||||
// stopped is more interesting than one that was not.
|
||||
func TestOnResult_IsToldWhenTheRuleRefuses(t *testing.T) {
|
||||
var seen []told
|
||||
app := zip.New(zip.Config{AppName: "restest2", DisableStartupMessage: true})
|
||||
var ran bool
|
||||
zip.Post(app, "/v1/things", func(_ context.Context, in *resIn) (*resOut, error) {
|
||||
ran = true
|
||||
return &resOut{OK: true}, nil
|
||||
}, zip.WithOperationID("createThing"))
|
||||
app.Authorize(func(_ context.Context, _ zip.Op, _ any) (zip.Decision, error) {
|
||||
return zip.Decision{Effect: zip.Deny, Clause: "none", Reason: "no"}, nil
|
||||
})
|
||||
app.OnResult(func(_ context.Context, op zip.Op, err error) {
|
||||
seen = append(seen, told{op: op.OperationID, err: err})
|
||||
})
|
||||
if err := app.Build(); err != nil {
|
||||
t.Fatalf("Build: %v", err)
|
||||
}
|
||||
|
||||
b, _ := json.Marshal(map[string]any{})
|
||||
req := httptest.NewRequest("POST", "/v1/things", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
resp, err := app.Fiber().Test(req)
|
||||
if err != nil {
|
||||
t.Fatalf("REST: %v", err)
|
||||
}
|
||||
_ = resp.Body.Close()
|
||||
|
||||
if ran {
|
||||
t.Fatal("the handler ran despite a Deny")
|
||||
}
|
||||
if len(seen) != 1 || seen[0].err == nil {
|
||||
t.Fatalf("a refused op told the hook %+v, want one entry carrying the refusal", seen)
|
||||
}
|
||||
}
|
||||
|
||||
// NO HOOK, NO COST, NO CHANGE. The default is unchanged for every app that
|
||||
// declares none.
|
||||
func TestOnResult_NilIsSilent(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "restest3", DisableStartupMessage: true})
|
||||
zip.Post(app, "/v1/things", func(_ context.Context, in *resIn) (*resOut, error) {
|
||||
return &resOut{OK: true}, nil
|
||||
}, zip.WithOperationID("createThing"))
|
||||
if err := app.Build(); err != nil {
|
||||
t.Fatalf("Build: %v", err)
|
||||
}
|
||||
b, _ := json.Marshal(map[string]any{})
|
||||
req := httptest.NewRequest("POST", "/v1/things", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
resp, err := app.Fiber().Test(req)
|
||||
if err != nil {
|
||||
t.Fatalf("REST: %v", err)
|
||||
}
|
||||
if resp.StatusCode != 200 {
|
||||
t.Fatalf("status = %d, want 200", resp.StatusCode)
|
||||
}
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
@@ -72,6 +72,10 @@ type registeredOp struct {
|
||||
// document asks it once — so "is this op governed" has one answer, and a
|
||||
// gated op cannot publish a contract its seam does not keep.
|
||||
rule func() Authorizer
|
||||
// result is the [App.OnResult] hook in force over this op, asked the same way
|
||||
// and for the same reason as rule: composition settles it at build, and the
|
||||
// op was registered before that.
|
||||
result func() func(context.Context, Op, error)
|
||||
invoke func(ctx context.Context, dec decoder, rawIn []byte, query, path map[string]string, header func(string) string) (any, error)
|
||||
// direct is invoke with the decoding removed: the In arrives as the *In the
|
||||
// caller already holds. It exists for the one transport that is not a
|
||||
@@ -618,6 +622,7 @@ func registerTyped[In, Out any](on OpTarget, method, path string, fn TypedHandle
|
||||
}
|
||||
op.readsHeaders = len(headerFields(op.InType)) > 0
|
||||
op.rule = app.rule
|
||||
op.result = app.result
|
||||
|
||||
// The op's stable identity, resolved once (after opts) and handed to the
|
||||
// authorizer on every invoke — REST and MCP alike.
|
||||
@@ -627,7 +632,26 @@ func registerTyped[In, Out any](on OpTarget, method, path string, fn TypedHandle
|
||||
// fn, return Out (or a literal nil for a void result). Both seams below end
|
||||
// here, so no way of reaching this op can skip a check another way makes. A
|
||||
// nil *Out becomes a nil `any`.
|
||||
run := func(ctx context.Context, in *In) (any, error) {
|
||||
run := func(ctx context.Context, in *In) (out any, err error) {
|
||||
// TOLD ONCE, ON EVERY WAY OUT. This is the one contract every projection
|
||||
// funnels through — REST, an MCP tools/call, the call plane, the graph and
|
||||
// an in-process invoke — so a hook here is told about all of them, and a
|
||||
// refusal is told about as surely as a success: [Deny] returns through the
|
||||
// same named err this reads.
|
||||
//
|
||||
// It is told the OPERATION and the OUTCOME and nothing else. Not the
|
||||
// output: a hook that could see the value would be a way for a response
|
||||
// body to reach a log or a trail, and the one thing every recorder in this
|
||||
// estate promises is that it never reads bodies. Not the input either, for
|
||||
// the same reason — [Authorizer] already sees that, before the handler, in
|
||||
// the one place a decision belongs.
|
||||
//
|
||||
// A held op reports a nil error. It did not fail; it did not run either,
|
||||
// and which of those happened is carried by the [Approval] the caller
|
||||
// receives rather than by this.
|
||||
if told := op.result(); told != nil {
|
||||
defer func() { told(ctx, meta, err) }()
|
||||
}
|
||||
if err := validate(in); err != nil {
|
||||
return nil, ErrBadRequest(err.Error())
|
||||
}
|
||||
@@ -653,14 +677,19 @@ func registerTyped[In, Out any](on OpTarget, method, path string, fn TypedHandle
|
||||
return d.approval(), nil
|
||||
}
|
||||
}
|
||||
out, err := fn(ctx, in)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
// The handler's result stays TYPED here. Binding it to the `any` return
|
||||
// instead would wrap a nil *Out in a non-nil interface, and the nil test
|
||||
// below — which is what a void op's 204 and a declared status both rest on
|
||||
// — would stop being true. Caught by opstatus_test.go, which is the only
|
||||
// reason the named returns above are safe to have.
|
||||
res, ferr := fn(ctx, in)
|
||||
if ferr != nil {
|
||||
return nil, ferr
|
||||
}
|
||||
if out == nil {
|
||||
if res == nil {
|
||||
return nil, nil
|
||||
}
|
||||
return out, nil
|
||||
return res, nil
|
||||
}
|
||||
|
||||
// The in-process seam: the caller already holds the *In, so there is nothing
|
||||
|
||||
@@ -261,6 +261,11 @@ type App struct {
|
||||
// every decoded request unauthorized. See [App.rule].
|
||||
authorizer Authorizer
|
||||
|
||||
// onResult, when set via OnResult, is told the outcome of every typed op this
|
||||
// app serves, however it was reached. nil defers to [App.over]. See
|
||||
// [App.result].
|
||||
onResult func(context.Context, Op, error)
|
||||
|
||||
// over is the App whose rule this one answers to: the nearest definition
|
||||
// ABOVE it in the composition that declared an Authorizer. Settled at build
|
||||
// by [adopt], which is the only place that knows — a definition is written
|
||||
|
||||
Reference in New Issue
Block a user