From 369b10322c0e1d032baeb702dab10ee3ed08fe03 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Tue, 29 Oct 2024 12:07:38 -0400 Subject: [PATCH] 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 Reviewed-by: Carlos Amedee Auto-Submit: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov --- internal/relui/web.go | 23 +++++++++++++++++++---- internal/relui/web_test.go | 19 +++++++++++++++---- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/internal/relui/web.go b/internal/relui/web.go index 5b544d7b..0bd581ca 100644 --- a/internal/relui/web.go +++ b/internal/relui/web.go @@ -260,7 +260,10 @@ func (s *Server) showWorkflowHandler(w http.ResponseWriter, r *http.Request, par return } 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) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return @@ -448,7 +451,11 @@ func (s *Server) retryTaskHandler(w http.ResponseWriter, r *http.Request, params } q := db.New(s.db) 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) return } @@ -476,7 +483,11 @@ func (s *Server) approveTaskHandler(w http.ResponseWriter, r *http.Request, para } q := db.New(s.db) 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) return } @@ -515,7 +526,11 @@ func (s *Server) stopWorkflowHandler(w http.ResponseWriter, r *http.Request, par } q := db.New(s.db) 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) return } diff --git a/internal/relui/web_test.go b/internal/relui/web_test.go index b63bb195..845d89d5 100644 --- a/internal/relui/web_test.go +++ b/internal/relui/web_test.go @@ -702,12 +702,23 @@ func TestServerStopWorkflow(t *testing.T) { rec := httptest.NewRecorder() worker := NewWorker(NewDefinitionHolder(), nil, nil) - wf := &workflow.Workflow{ID: wfID} - if err := worker.markRunning(wf, cancel); err != nil { - t.Fatalf("worker.markRunning(%v, %v) = %v, wanted no error", wf, cancel, err) + p := testDB(ctx, t) + q := db.New(p) + 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) resp := rec.Result()