internal/relui: serve 404 when workflow doesn't exist, fix DB tests

Relui is expected to serve 404 when a workflow that doesn't exist is
requested. Its tests, ones that run only when a database is available,
also expect that. Update a few places after CL 593915 accordingly to
serve 404 instead of 500, and update TestServerStopWorkflow to create
a workflow in the DB, since stopWorkflowHandler now fetches it as part
of checking ACLs.

For golang/go#68114.

Change-Id: I90f12a431b1f97d8be33d6404eb7e2064e50f688
Reviewed-on: https://go-review.googlesource.com/c/build/+/624076
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This commit is contained in:
Dmitri Shuralyov 2024-10-29 12:07:38 -04:00 коммит произвёл Gopher Robot
Родитель 30ecbc408c
Коммит 369b10322c
2 изменённых файлов: 34 добавлений и 8 удалений

Просмотреть файл

@ -260,7 +260,10 @@ func (s *Server) showWorkflowHandler(w http.ResponseWriter, r *http.Request, par
return return
} }
resp, err := s.buildShowWorkflowResponse(r.Context(), id) resp, err := s.buildShowWorkflowResponse(r.Context(), id)
if err != nil { if errors.Is(err, sql.ErrNoRows) || errors.Is(err, pgx.ErrNoRows) {
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
return
} else if err != nil {
log.Printf("showWorkflowHandler: %v", err) log.Printf("showWorkflowHandler: %v", err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return return
@ -448,7 +451,11 @@ func (s *Server) retryTaskHandler(w http.ResponseWriter, r *http.Request, params
} }
q := db.New(s.db) q := db.New(s.db)
workflow, err := q.Workflow(r.Context(), id) workflow, err := q.Workflow(r.Context(), id)
if err != nil { if errors.Is(err, sql.ErrNoRows) || errors.Is(err, pgx.ErrNoRows) {
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
return
} else if err != nil {
log.Printf("retryTaskHandler(_, _, %v): Workflow(%d): %v", params, id, err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return return
} }
@ -476,7 +483,11 @@ func (s *Server) approveTaskHandler(w http.ResponseWriter, r *http.Request, para
} }
q := db.New(s.db) q := db.New(s.db)
workflow, err := q.Workflow(r.Context(), id) workflow, err := q.Workflow(r.Context(), id)
if err != nil { if errors.Is(err, sql.ErrNoRows) || errors.Is(err, pgx.ErrNoRows) {
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
return
} else if err != nil {
log.Printf("approveTaskHandler(_, _, %v): Workflow(%d): %v", params, id, err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return return
} }
@ -515,7 +526,11 @@ func (s *Server) stopWorkflowHandler(w http.ResponseWriter, r *http.Request, par
} }
q := db.New(s.db) q := db.New(s.db)
workflow, err := q.Workflow(r.Context(), id) workflow, err := q.Workflow(r.Context(), id)
if err != nil { if errors.Is(err, sql.ErrNoRows) || errors.Is(err, pgx.ErrNoRows) {
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
return
} else if err != nil {
log.Printf("stopWorkflowHandler(_, _, %v): Workflow(%d): %v", params, id, err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return return
} }

Просмотреть файл

@ -702,12 +702,23 @@ func TestServerStopWorkflow(t *testing.T) {
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
worker := NewWorker(NewDefinitionHolder(), nil, nil) worker := NewWorker(NewDefinitionHolder(), nil, nil)
wf := &workflow.Workflow{ID: wfID} p := testDB(ctx, t)
if err := worker.markRunning(wf, cancel); err != nil { q := db.New(p)
t.Fatalf("worker.markRunning(%v, %v) = %v, wanted no error", wf, cancel, err) wf := db.CreateWorkflowParams{
ID: wfID,
Params: nullString(`{"farewell": "bye", "greeting": "hello"}`),
Name: nullString("echo"),
CreatedAt: time.Now().Add(-1 * time.Hour),
UpdatedAt: time.Now().Add(-1 * time.Hour),
}
if _, err := q.CreateWorkflow(ctx, wf); err != nil {
t.Fatalf("CreateWorkflow(%v) = %v, wanted no error", wf, err)
}
if err := worker.markRunning(&workflow.Workflow{ID: wfID}, cancel); err != nil {
t.Fatalf("worker.markRunning(%q) = %v, wanted no error", wfID, err)
} }
s := NewServer(testDB(ctx, t), worker, nil, SiteHeader{}, nil, nil) s := NewServer(p, worker, nil, SiteHeader{}, nil, nil)
s.m.ServeHTTP(rec, req) s.m.ServeHTTP(rec, req)
resp := rec.Result() resp := rec.Result()