Fail interrupted groundings at boot and bound hand grounding
At boot, after the job runner fails the jobs a previous process left running, FailInterruptedGroundings sets every dinner still pending with no ground_dinner job queued or running to failed (interrupted). A crash mid-job, or between saving a dinner and enqueuing its job, no longer leaves the dinner pending forever. A search failure on recipe-candidates or ground now logs only the reason code search_failed, never the wrapped error, which may quote the provider; the response stays redacted. When the grounding job cannot be enqueued, POST /dinners answers 201 with the saved dinner failed (grounding_error) and grounding_job_id null, instead of a 500 that hid the dinner it had created. Grounding a candidate or link by hand runs under a 90-second deadline (fetch, photo, normalize_recipe); past it the request answers 504 ground_timeout and the dinner is unchanged. Maestro task: 0cbaa5bf-8be4-4cbd-aab2-891e7f1bc2b5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -71,7 +71,8 @@ func getDinners(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
|
||||
|
||||
// postDinner adds a Regular, recording the session's actor as updated_by,
|
||||
// and answers 201 with it and its grounding_job_id without waiting for the
|
||||
// job.
|
||||
// job. When the job cannot be enqueued it still answers 201, with the dinner
|
||||
// grounding_status failed and grounding_job_id null.
|
||||
func postDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
actor, ok := actorOrFail(w, r, logger)
|
||||
@@ -198,7 +199,8 @@ func actorOrFail(w http.ResponseWriter, r *http.Request, logger *slog.Logger) (a
|
||||
|
||||
// writeDinnerError maps a dinners.Service error to its API error. A page
|
||||
// that cannot ground the dinner is 422 ungroundable; recipe search not set
|
||||
// up is 409 not_configured; a search or model failure is 502.
|
||||
// up is 409 not_configured; a search or model failure is 502; grounding by
|
||||
// hand that runs past dinners.GroundTimeout is 504 ground_timeout.
|
||||
func writeDinnerError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) {
|
||||
var invalid *dinners.ValidationError
|
||||
var dup *dinners.DuplicateError
|
||||
@@ -211,11 +213,16 @@ func writeDinnerError(w http.ResponseWriter, r *http.Request, logger *slog.Logge
|
||||
case errors.Is(err, recipes.ErrSearchNotConfigured):
|
||||
writeError(w, r, logger, http.StatusConflict, "not_configured", "recipe search is not configured")
|
||||
case errors.Is(err, recipes.ErrSearchFailed):
|
||||
logger.WarnContext(r.Context(), action, "error", err)
|
||||
// The error may quote the search provider's response, so only the
|
||||
// reason is logged.
|
||||
logger.WarnContext(r.Context(), action, "reason", "search_failed")
|
||||
writeError(w, r, logger, http.StatusBadGateway, "search_failed", "recipe search failed")
|
||||
case errors.Is(err, recipes.ErrNormalizeFailed):
|
||||
logger.WarnContext(r.Context(), action, "reason", "normalize_recipe failed")
|
||||
writeError(w, r, logger, http.StatusBadGateway, "ai_failed", "the AI provider failed to read the recipe")
|
||||
case errors.Is(err, dinners.ErrGroundTimeout):
|
||||
logger.WarnContext(r.Context(), action, "reason", "ground_timeout")
|
||||
writeError(w, r, logger, http.StatusGatewayTimeout, "ground_timeout", "grounding the recipe took too long; try again")
|
||||
case errors.As(err, &invalid):
|
||||
writeError(w, r, logger, http.StatusBadRequest, "invalid_dinner", invalid.Error())
|
||||
case errors.As(err, &dup):
|
||||
|
||||
@@ -1,12 +1,14 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"testing/fstest"
|
||||
|
||||
@@ -39,6 +41,26 @@ type dinnersEnv struct {
|
||||
pages *recipestest.Pages
|
||||
images *recipestest.Images
|
||||
model *llm.Fake
|
||||
// logs holds everything the router and services log.
|
||||
logs *logBuffer
|
||||
}
|
||||
|
||||
// logBuffer is a bytes.Buffer safe for concurrent log writes.
|
||||
type logBuffer struct {
|
||||
mu sync.Mutex
|
||||
buf bytes.Buffer
|
||||
}
|
||||
|
||||
func (b *logBuffer) Write(p []byte) (int, error) {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
return b.buf.Write(p)
|
||||
}
|
||||
|
||||
func (b *logBuffer) String() string {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
return b.buf.String()
|
||||
}
|
||||
|
||||
func newDinnersEnv(t *testing.T) *dinnersEnv {
|
||||
@@ -50,8 +72,9 @@ func newDinnersEnv(t *testing.T) *dinnersEnv {
|
||||
pages: &recipestest.Pages{Bodies: map[string][]byte{}},
|
||||
images: &recipestest.Images{JPEGs: map[string][]byte{}},
|
||||
model: &llm.Fake{},
|
||||
logs: &logBuffer{},
|
||||
}
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
logger := slog.New(slog.NewTextHandler(e.logs, nil))
|
||||
runner, err := jobs.NewRunner(st, logger, 1)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -162,12 +163,21 @@ func TestGetRecipeCandidates(t *testing.T) {
|
||||
func TestGetRecipeCandidatesSearchFails(t *testing.T) {
|
||||
t.Parallel()
|
||||
e := newDinnersEnv(t)
|
||||
e.search.Err = recipes.ErrSearchRefused
|
||||
// The provider's text must reach neither the response nor the logs.
|
||||
const leak = "provider-said-token-abc123"
|
||||
e.search.Err = fmt.Errorf("%w: %s", recipes.ErrSearchRefused, leak)
|
||||
id := createDinner(t, e.h, `{"name":"Chicken curry"}`)
|
||||
rec, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "")
|
||||
if rec.Code != http.StatusBadGateway || errorCode(body) != "search_failed" {
|
||||
t.Errorf("GET = %d %v, want 502 search_failed", rec.Code, body)
|
||||
}
|
||||
if strings.Contains(rec.Body.String(), leak) {
|
||||
t.Errorf("response quotes the provider: %s", rec.Body)
|
||||
}
|
||||
logs := e.logs.String()
|
||||
if strings.Contains(logs, leak) || !strings.Contains(logs, "reason=search_failed") {
|
||||
t.Errorf("logs = %q, want reason=search_failed and no provider text", logs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPostGroundViaCandidate(t *testing.T) {
|
||||
@@ -271,3 +281,23 @@ func TestPostGroundRejects(t *testing.T) {
|
||||
t.Errorf("unknown dinner = %d %v, want 404", rec.Code, body)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPostDinnerEnqueueFailureAnswersFailedDinner(t *testing.T) {
|
||||
t.Parallel()
|
||||
e := newDinnersEnv(t)
|
||||
if _, err := e.st.Writer.ExecContext(t.Context(),
|
||||
`CREATE TRIGGER refuse_jobs BEFORE INSERT ON jobs BEGIN SELECT RAISE(ABORT, 'no jobs'); END`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rec, created := callDinners(t, e.h, http.MethodPost, dinnersPath, `{"name":"Chicken curry"}`)
|
||||
if rec.Code != http.StatusCreated {
|
||||
t.Fatalf("POST = %d: %s", rec.Code, rec.Body)
|
||||
}
|
||||
id, _ := created["id"].(string)
|
||||
if id == "" || created["grounding_status"] != "failed" || created["grounding_error"] != "grounding_error" || created["grounding_job_id"] != nil {
|
||||
t.Fatalf("created = %v, want its id, failed (grounding_error) and a null grounding_job_id", created)
|
||||
}
|
||||
if got := listedDinner(t, e.h, id); got["grounding_status"] != "failed" {
|
||||
t.Errorf("listed = %v, want failed", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user