Skip to content

Commit 639c35e

Browse files
[CAP-9161] return clear error when locally running a nonexistent task (#320)
Fix error propagation when starting a task run for an invalid task identifier on the local dev server. GitOrigin-RevId: 43a9c557a21d493e8b79bd565bc724d73198ab6d
1 parent c87b36d commit 639c35e

4 files changed

Lines changed: 75 additions & 8 deletions

File tree

pkg/workflows/apiserver/server.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func handleError(w http.ResponseWriter, err error, statusCode int) {
3737
log.Println("error marshalling error", err)
3838
return
3939
}
40-
w.Write(errJSON)
40+
_, _ = w.Write(errJSON)
4141
}
4242

4343
func Start(handler *ServerHandler, port int) (*http.Server, error) {
@@ -153,7 +153,6 @@ func (h *ServerHandler) RunTask(w http.ResponseWriter, r *http.Request) {
153153
if err != nil {
154154
if _, ok := err.(*orchestrator.TaskNotFoundError); ok {
155155
handleError(w, err, http.StatusNotFound)
156-
w.Write([]byte(err.Error()))
157156
return
158157
}
159158

@@ -175,16 +174,12 @@ func (h *ServerHandler) RunTask(w http.ResponseWriter, r *http.Request) {
175174
func (h *ServerHandler) ListTaskRuns(w http.ResponseWriter, r *http.Request) {
176175
w.Header().Set("Content-Type", "application/json")
177176

178-
var params client.ListTaskRunsParams
179-
180-
params.TaskId = pointers.From([]string{r.URL.Query().Get("taskId")})
181-
if params.TaskId == nil {
177+
taskID := r.URL.Query().Get("taskId")
178+
if taskID == "" {
182179
handleError(w, fmt.Errorf("taskId is required"), http.StatusBadRequest)
183-
w.Write([]byte("taskId is required"))
184180
return
185181
}
186182

187-
taskID := (*params.TaskId)[0]
188183
json.NewEncoder(w).Encode(internal.ListTaskRuns(h.taskStore, taskID))
189184
}
190185

pkg/workflows/orchestrator/coordinator.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ type StatusReporter interface {
2222
TaskRunning(taskRun *store.TaskRun)
2323
TaskCompleted(taskRun *store.TaskRun)
2424
TaskFailed(taskRun *store.TaskRun)
25+
TaskNotFound(taskIdentifier string)
2526
}
2627

2728
type Coordinator struct {
@@ -114,6 +115,7 @@ func (c *Coordinator) StartTask(ctx context.Context, taskIdentifier string, inpu
114115

115116
task := c.store.GetTask(taskIdentifier)
116117
if task == nil {
118+
c.statusReporter.TaskNotFound(taskIdentifier)
117119
return nil, &TaskNotFoundError{TaskIdentifier: taskIdentifier}
118120
}
119121

pkg/workflows/orchestrator/coordinator_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,76 @@ func (noopStatusReporter) TaskEnqueued(taskRun *store.TaskRun) {}
3939
func (noopStatusReporter) TaskRunning(taskRun *store.TaskRun) {}
4040
func (noopStatusReporter) TaskCompleted(taskRun *store.TaskRun) {}
4141
func (noopStatusReporter) TaskFailed(taskRun *store.TaskRun) {}
42+
func (noopStatusReporter) TaskNotFound(taskIdentifier string) {}
4243

4344
func (f *fakeServerFactory) NewHandler(socket net.Listener, input taskserver.GetInput200JSONResponse, getSubtaskResultFunc taskserver.GetSubtaskResultFunc, startSubtaskFunc taskserver.StartSubtaskFunc) *taskserver.ServerHandler {
4445
return f.newHandler(socket, input, getSubtaskResultFunc, startSubtaskFunc)
4546
}
4647

48+
type recordingStatusReporter struct {
49+
noopStatusReporter
50+
notFoundIdentifier string
51+
}
52+
53+
func (r *recordingStatusReporter) TaskNotFound(taskIdentifier string) {
54+
r.notFoundIdentifier = taskIdentifier
55+
}
56+
57+
func TestStartTaskNotFound(t *testing.T) {
58+
ctx := context.Background()
59+
60+
s := store.NewTaskStore()
61+
62+
tasks := []taskserver.Task{
63+
{
64+
Name: "test-task",
65+
},
66+
}
67+
68+
postTasksChan := make(chan taskserver.PostRegisterTasksRequestObject, 1)
69+
postTasksChan <- taskserver.PostRegisterTasksRequestObject{
70+
Body: &taskserver.PostRegisterTasksJSONRequestBody{
71+
Tasks: tasks,
72+
},
73+
}
74+
75+
socketTracker, err := orchestrator.NewSocketTracker(ctx)
76+
require.NoError(t, err)
77+
78+
reporter := &recordingStatusReporter{}
79+
coordinator := orchestrator.NewCoordinator(
80+
ctx,
81+
s,
82+
&fakeSdkExec{
83+
startService: func(ctx context.Context, taskRunID string, socket string, mode orchestrator.Mode) (func() error, <-chan error, error) {
84+
return func() error { return nil }, neverExits(), nil
85+
},
86+
},
87+
socketTracker,
88+
&fakeServerFactory{
89+
newHandler: func(socket net.Listener, input taskserver.GetInput200JSONResponse, getSubtaskResultFunc taskserver.GetSubtaskResultFunc, startSubtaskFunc taskserver.StartSubtaskFunc) *taskserver.ServerHandler {
90+
return &taskserver.ServerHandler{
91+
Socket: socket,
92+
Input: input,
93+
Channels: taskserver.ServerChannels{
94+
PostCallback: make(chan taskserver.PostCallbackRequestObject),
95+
PostTasks: postTasksChan,
96+
},
97+
}
98+
},
99+
},
100+
reporter,
101+
)
102+
103+
_, err = coordinator.StartTask(ctx, "nonexistent-task", []byte{}, nil)
104+
require.Error(t, err)
105+
106+
var taskNotFoundErr *orchestrator.TaskNotFoundError
107+
require.ErrorAs(t, err, &taskNotFoundErr)
108+
require.Equal(t, "nonexistent-task", taskNotFoundErr.TaskIdentifier)
109+
require.Equal(t, "nonexistent-task", reporter.notFoundIdentifier)
110+
}
111+
47112
func TestStartTask(t *testing.T) {
48113
ctx := context.Background()
49114

pkg/workflows/orchestrator/statusreporter.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ func (r *PrintStatusReporter) TaskFailed(taskRun *store.TaskRun) {
8585
r.print(r.formatWithDuration("%s %s: %s%s", taskRun), r.taskLabel(taskRun), status, r.describeTaskRun(taskRun), details)
8686
}
8787

88+
func (r *PrintStatusReporter) TaskNotFound(taskIdentifier string) {
89+
status := renderstyle.Status.Foreground(renderstyle.ColorError).Render("Not found")
90+
r.print("Task %s: %s", status, taskIdentifier)
91+
}
92+
8893
func (r *PrintStatusReporter) print(format string, args ...any) {
8994
if r.printFn == nil {
9095
return

0 commit comments

Comments
 (0)