diff --git a/cmd/mavend/ecosystem.go b/cmd/mavend/ecosystem.go index 2e1a9bf..8ff69e0 100644 --- a/cmd/mavend/ecosystem.go +++ b/cmd/mavend/ecosystem.go @@ -12,6 +12,7 @@ import ( "log" "net/http" "net/url" + "strings" "time" hexisclient "github.com/kami/hexis/pkg/client" @@ -179,6 +180,54 @@ func httpError(service, op string, status int) *ecosystemError { } } +// hexisStatusTexts maps the http.StatusText spelling back to its code, for the +// failure statuses a Hexis call can plausibly answer with. It is the inverse of +// what the vendored client threw away. +var hexisStatusTexts = func() map[string]int { + codes := []int{ + http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, + http.StatusNotFound, http.StatusMethodNotAllowed, http.StatusNotAcceptable, + http.StatusRequestTimeout, http.StatusConflict, http.StatusGone, + http.StatusUnprocessableEntity, http.StatusUpgradeRequired, + http.StatusTooManyRequests, http.StatusInternalServerError, + http.StatusNotImplemented, http.StatusBadGateway, + http.StatusServiceUnavailable, http.StatusGatewayTimeout, + } + m := make(map[string]int, len(codes)) + for _, c := range codes { + m[http.StatusText(c)] = c + } + return m +}() + +// hexisError re-wraps an error from the vendored Hexis client as an +// *ecosystemError, so a Hexis failure classifies the same way a Nexus or Praxis +// one does and ecosystemGap can tell a refused credential from an outage. +// +// This is a boundary adapter and it is not the fix anyone would choose. The +// Hexis client lives in another repository and returns +// fmt.Errorf("%s: %s", http.StatusText(status), body) for every status at or +// above 400, so the status text is the only signal that survives — the correct +// fix is a typed error carrying the code, and Maven cannot land it unilaterally +// (Vikunja #587, docs/plans/20-two-artifacts-and-neither-is-spring.md). Parsing +// here is bounded: the message's first colon-delimited segment is the status +// text verbatim, no status text contains a colon, and anything unrecognised — +// "do request: ...", "create request: ..." — is a transport failure and is left +// at status 0, which is exactly what Unreachable() means. +func hexisError(op string, err error) error { + if err == nil { + return nil + } + var ee *ecosystemError + if errors.As(err, &ee) { + return err + } + head, _, _ := strings.Cut(err.Error(), ": ") + return &ecosystemError{ + Service: "hexis", Op: op, Status: hexisStatusTexts[head], Err: err, + } +} + type nexusClient struct { ecosystemHTTP } @@ -530,6 +579,7 @@ func (w *ecosystemWiring) discoverCapabilities(ctx context.Context, entityID str } caps, err := w.hexis.Capabilities(ctx, entityID) if err != nil { + err = hexisError("capabilities", err) log.Printf("ecosystem: hexis capabilities error: %v", err) return nil, err } @@ -557,7 +607,10 @@ func (w *ecosystemWiring) executeCapability(ctx context.Context, capabilityID, t exec, err := w.hexis.Execute(ctx, req) if err != nil { - return correlationID, fmt.Errorf("execute: %w", err) + // A classified dependency failure. The two returns below are NOT: an + // execution that ran and failed is the command failing, not Hexis + // degrading, and it keeps its plain error so the caller says so. + return correlationID, hexisError("execute", err) } if exec.Status == "succeeded" { return correlationID, nil diff --git a/cmd/mavend/ecosystem_acts.go b/cmd/mavend/ecosystem_acts.go index 4c7a852..94f41c2 100644 --- a/cmd/mavend/ecosystem_acts.go +++ b/cmd/mavend/ecosystem_acts.go @@ -551,6 +551,14 @@ func unauthorizedEcosystemError(err error) bool { return errors.As(err, &ee) && ee.Unauthorized() } +// isEcosystemError reports a failure that belongs to the service rather than to +// what was asked of it: a call that never landed, or one the far side refused. +// It separates "Hexis is down" from "the restart failed". +func isEcosystemError(err error) bool { + var ee *ecosystemError + return errors.As(err, &ee) +} + // traceErrorFields describes an ecosystemError for a trace without leaking the // payload: the HTTP status and the failure class, nothing else. func traceErrorFields(err error) map[string]any { @@ -777,6 +785,14 @@ func (h *reactiveHandler) execHexis(ctx context.Context, capID, capName, entityI mergeFields(traceErrorFields(err), map[string]any{ "entity_id": entityID, "capability": capName, "causation_id": causationID, })) + // Hexis never answering, or answering "no", is a gap in Hexis and is + // named as one — a refused token said "не получилось выполнить команду" + // here and sent him to debug a capability that was never reached + // (Vikunja #587). An execution that genuinely ran and failed is not an + // ecosystemError and keeps the command-level line. + if isEcosystemError(err) { + return ecosystemGap(serviceHexis, err) + } return phraser.A(phraser.ActFailEntity, map[string]string{"name": displayName}) } // One record per hop: the second write this used to make said the same diff --git a/cmd/mavend/ecosystem_gap_test.go b/cmd/mavend/ecosystem_gap_test.go new file mode 100644 index 0000000..e5797a0 --- /dev/null +++ b/cmd/mavend/ecosystem_gap_test.go @@ -0,0 +1,159 @@ +package main + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/phraser" +) + +// A refused credential and an outage are different answers, and on the Hexis +// path only one of them used to be said. These tests pin the difference at both +// Hexis sites: the discovery hop and the execute hop (Vikunja #587). The Praxis +// half of the same defect is in praxis_gap_test.go. +// +// unreachableURL is a port nothing listens on, which is what "the service is +// down" looks like from inside a call: the connection is refused, no HTTP +// answer is ever produced, and ecosystemError.Unreachable() is true. +const unreachableURL = "http://127.0.0.1:1" + +func denied(service, reply string) bool { + return phraser.IsA(phraser.EcoDenied, serviceVars(service), reply) +} + +func down(service, reply string) bool { + return phraser.IsA(phraser.EcoDown, serviceVars(service), reply) +} + +// hexisGapHandler wires a handler whose Nexus resolves cleanly and whose Hexis +// is the caller's to break. hexisURL is taken separately so a test can point it +// at a dead port. +func hexisGapHandler(t *testing.T, nexusURL, hexisURL string) *reactiveHandler { + t.Helper() + st := newTestStore(t) + now := time.Now() + return &reactiveHandler{ + api: ipc.NewStoreAPI(st), + dataStore: st, + now: func() time.Time { return now }, + ecosystem: stubEcosystem(nexusURL, hexisURL), + } +} + +// TestHexisDiscovery401IsDeniedNotDown — the discovery hop. +// +// The vendored Hexis client returns a plain fmt.Errorf for every status at or +// above 400, so errors.As for *ecosystemError never matched and every failure +// fell through to the outage line. "Hexis is down" for a rejected token sends +// him to inspect a service that is running fine. +func TestHexisDiscovery401IsDeniedNotDown(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := newFakeHexis(t, caps, fixtureHexisExecuted("exec_1", "succeeded")) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + hexis.SetFault(401) + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !denied(serviceHexis, reply) { + t.Fatalf("401 from hexis discovery: got %q, want the denied line naming Hexis", reply) + } + if !strings.Contains(reply, serviceHexis) { + t.Errorf("reply does not name Hexis: %q", reply) + } +} + +// TestHexisDiscoveryOutageIsDownNotDenied — the other half of the same fork. +// Without this the fix could pass by calling everything a refused credential. +func TestHexisDiscoveryOutageIsDownNotDenied(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + h := hexisGapHandler(t, nexus.URL, unreachableURL) + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !down(serviceHexis, reply) { + t.Fatalf("connection refused from hexis: got %q, want the outage line naming Hexis", reply) + } + if denied(serviceHexis, reply) { + t.Error("an outage must not be reported as a refused credential") + } +} + +// TestHexisExecute401IsDeniedNotCommandFailure — the execute hop, which did not +// consult ecosystemGap at all and named neither the service nor the cause. +func TestHexisExecute401IsDeniedNotCommandFailure(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := newFakeHexis(t, caps, fixtureHexisExecuted("exec_1", "succeeded")) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + // Discovery stays healthy; only the execute endpoint refuses. A blanket + // fault would never reach the site under test. + hexis.SetRouteFault("/api/v1/execute", 401) + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !denied(serviceHexis, reply) { + t.Fatalf("401 from hexis execute: got %q, want the denied line naming Hexis", reply) + } +} + +// TestHexisExecuteOutageIsDown — same site, the other classification. +// +// Discovery and execution share one base URL, so the outage has to be scoped to +// the execute endpoint rather than to the server: it answers capabilities +// normally and drops the connection on execute, which is what the client sees +// when the far side dies mid-call. That produces no HTTP status at all, which is +// what Unreachable() means. +func TestHexisExecuteOutageIsDown(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/execute" { + conn, _, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Errorf("hijack: %v", err) + return + } + conn.Close() + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(caps)) + })) + t.Cleanup(hexis.Close) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !down(serviceHexis, reply) { + t.Fatalf("dropped connection on hexis execute: got %q, want the outage line", reply) + } + if denied(serviceHexis, reply) { + t.Error("an outage must not be reported as a refused credential") + } +} + +// TestHexisExecutionFailedStaysCommandFailure — the boundary of the fix. Hexis +// answering 200 with a failed execution is the command failing, not Hexis +// degrading, and it must keep the command-level line rather than accusing a +// healthy service of being down. +func TestHexisExecutionFailedStaysCommandFailure(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := newFakeHexis(t, caps, fixtureHexisExecutionFailed("exec_1", "unit refused to start")) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if down(serviceHexis, reply) || denied(serviceHexis, reply) { + t.Fatalf("a failed execution must not be reported as an ecosystem gap, got %q", reply) + } + if !phraser.IsA(phraser.ActFailEntity, map[string]string{"name": muzickIndexer}, reply) { + t.Fatalf("want the command-failure line, got %q", reply) + } +}