From df7bf4a0332872610a34697156cc4070ff561453 Mon Sep 17 00:00:00 2001 From: david may <1301201+wass3r@users.noreply.github.com> Date: Fri, 9 Aug 2024 14:19:40 +0000 Subject: [PATCH] fix(webhook): only send one response to client (#1154) * fix(webhook): only send one response to client during webhook processing some of the helper function can set a status response for the client while the callee is also doing so. this can cause a warning that the status is being set multiple times when they differ. this change puts the responsibility on the caller of the helper functions to return a response to the client - once. * improve wording --------- Co-authored-by: Easton Crupper <65553218+ecrupper@users.noreply.github.com> --- api/webhook/post.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/api/webhook/post.go b/api/webhook/post.go index 89b7907ac..e8ce98ddd 100644 --- a/api/webhook/post.go +++ b/api/webhook/post.go @@ -552,6 +552,8 @@ func PostWebhook(c *gin.Context) { // handleRepositoryEvent is a helper function that processes repository events from the SCM and updates // the database resources with any relevant changes resulting from the event, such as name changes, transfers, etc. +// +// the caller is responsible for returning errors to the client. func handleRepositoryEvent(ctx context.Context, c *gin.Context, m *internal.Metadata, h *library.Hook, r *types.Repo) (*types.Repo, error) { l := c.MustGet("logger").(*logrus.Entry) @@ -664,6 +666,8 @@ func handleRepositoryEvent(ctx context.Context, c *gin.Context, m *internal.Meta // queries the database for the repo that matches that name and org, and updates // that repo to its new name in order to preserve it. It also updates the secrets // associated with that repo as well as build links for the UI. +// +// the caller is responsible for returning errors to the client. func RenameRepository(ctx context.Context, h *library.Hook, r *types.Repo, c *gin.Context, m *internal.Metadata) (*types.Repo, error) { l := c.MustGet("logger").(*logrus.Entry) @@ -692,7 +696,6 @@ func RenameRepository(ctx context.Context, h *library.Hook, r *types.Repo, c *gi lastHook, err := database.FromContext(c).LastHookForRepo(ctx, dbR) if err != nil { retErr := fmt.Errorf("unable to get last hook for repo %s: %w", r.GetFullName(), err) - util.HandleError(c, http.StatusInternalServerError, retErr) h.SetStatus(constants.StatusFailure) h.SetError(retErr.Error()) @@ -796,7 +799,6 @@ func RenameRepository(ctx context.Context, h *library.Hook, r *types.Repo, c *gi dbR, err = database.FromContext(c).UpdateRepo(ctx, dbR) if err != nil { retErr := fmt.Errorf("%s: failed to update repo %s/%s", baseErr, dbR.GetOrg(), dbR.GetName()) - util.HandleError(c, http.StatusBadRequest, retErr) h.SetStatus(constants.StatusFailure) h.SetError(retErr.Error())