Merge pull request #451 from alessandro-sorint/fix-apirunbygroup

runservice: return right error on not existing run
agola-io · Oct 20, 2023 · 52b37fe · 52b37fe
2 parents f851837 + ca84e0c
commit 52b37fe
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 17 deletions.
diff --git a/internal/services/runservice/api/api.go b/internal/services/runservice/api/api.go
@@ -488,6 +488,18 @@ func NewRunByGroupHandler(log zerolog.Logger, d *db.DB, ah *action.ActionHandler
 }
 
 func (h *RunByGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	res, err := h.do(w, r)
+	if util.HTTPError(w, err) {
+		h.log.Err(err).Send()
+		return
+	}
+
+	if err := util.HTTPResponse(w, http.StatusOK, res); err != nil {
+		h.log.Err(err).Send()
+	}
+}
+
+func (h *RunByGroupHandler) do(w http.ResponseWriter, r *http.Request) (*rsapitypes.RunResponse, error) {
 	ctx := r.Context()
 	vars := mux.Vars(r)
 
@@ -496,22 +508,20 @@ func (h *RunByGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	group, err := url.PathUnescape(vars["group"])
 	if err != nil {
-		util.HTTPError(w, util.NewAPIError(util.ErrBadRequest, errors.Errorf("group is empty")))
-		return
+		return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("group is empty"))
 	}
 
 	runCounterStr := vars["runcounter"]
 
 	var runCounter uint64
 	if runCounterStr == "" {
-		util.HTTPError(w, util.NewAPIError(util.ErrBadRequest, errors.Errorf("runcounter is empty")))
+		return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("runcounter is empty"))
 	}
 	if runCounterStr != "" {
 		var err error
 		runCounter, err = strconv.ParseUint(runCounterStr, 10, 64)
 		if err != nil {
-			util.HTTPError(w, util.NewAPIError(util.ErrBadRequest, errors.Wrapf(err, "cannot parse runcounter")))
-			return
+			return nil, util.NewAPIError(util.ErrBadRequest, errors.Wrapf(err, "cannot parse runcounter"))
 		}
 	}
 
@@ -539,23 +549,19 @@ func (h *RunByGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return errors.WithStack(err)
 	})
 	if err != nil {
-		h.log.Err(err).Send()
-		http.Error(w, err.Error(), http.StatusInternalServerError)
-		return
+		return nil, errors.WithStack(err)
 	}
 	if run == nil {
-		util.HTTPError(w, util.NewAPIError(util.ErrNotExist, errors.Errorf("run for group %q with counter %d doesn't exist", group, runCounter)))
+		return nil, util.NewAPIError(util.ErrNotExist, errors.Errorf("run for group %q with counter %d doesn't exist", group, runCounter))
 	}
 
 	if rc == nil {
-		util.HTTPError(w, util.NewAPIError(util.ErrNotExist, errors.Errorf("run config for run with id %q doesn't exist", run.ID)))
-		return
+		return nil, util.NewAPIError(util.ErrNotExist, errors.Errorf("run config for run with id %q doesn't exist", run.ID))
 	}
 
 	cgts, err := types.MarshalChangeGroupsUpdateToken(cgt)
 	if err != nil {
-		http.Error(w, err.Error(), http.StatusInternalServerError)
-		return
+		return nil, errors.WithStack(err)
 	}
 
 	res := &rsapitypes.RunResponse{
@@ -564,10 +570,7 @@ func (h *RunByGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		ChangeGroupsUpdateToken: cgts,
 	}
 
-	if err := util.HTTPResponse(w, http.StatusOK, res); err != nil {
-		h.log.Err(err).Send()
-	}
-
+	return res, nil
 }
 
 const (

diff --git a/tests/setup_test.go b/tests/setup_test.go
@@ -5780,6 +5780,39 @@ func TestGetProjectRuns(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("test get not existing run", func(t *testing.T) {
+		t.Parallel()
+
+		dir := t.TempDir()
+
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+
+		sc := setup(ctx, t, dir, withGitea(true))
+		defer sc.stop()
+
+		giteaAPIURL := fmt.Sprintf("http://%s:%s", sc.gitea.HTTPListenAddress, sc.gitea.HTTPPort)
+
+		giteaToken, token := createLinkedAccount(ctx, t, sc.gitea, sc.config)
+
+		giteaClient, err := gitea.NewClient(giteaAPIURL, gitea.SetToken(giteaToken))
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		gwClient := gwclient.NewClient(sc.config.Gateway.APIExposedURL, token)
+
+		_, project := createProject(ctx, t, giteaClient, gwClient)
+
+		_, _, err = gwClient.GetProjectRun(ctx, project.ID, 1)
+		if err == nil {
+			t.Fatalf("expected error %v, got nil err", remoteErrorNotExist)
+		}
+		if err.Error() != remoteErrorNotExist {
+			t.Fatalf("expected err %v, got err: %v", remoteErrorNotExist, err)
+		}
+	})
 }
 
 func TestRunEventsNotification(t *testing.T) {