Skip to content

Commit

Permalink
internal/relui: serve 404 when workflow doesn't exist, fix DB tests
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dmitshur authored and gopherbot committed Nov 4, 2024
1 parent 30ecbc4 commit 369b103
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 8 deletions.
23 changes: 19 additions & 4 deletions internal/relui/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 15 additions & 4 deletions internal/relui/web_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down

0 comments on commit 369b103

Please sign in to comment.