Resolve: "Pipelines: When we retry a pipeline 2 requests are made to the pipelines endpoint"

What does this MR do?

When the user clicks on retry or cancel, server should send 204 response.

Changes made:

• Frontend makes HTTP request with content-type: 'application/json'
• Backend send 204 for JSON

Why was this MR needed?

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

• Changelog entry added, if necessary
• Documentation created/updated
• API support added
• Tests
  • Added for this feature/bug
  • All builds are passing
• Conform by the merge request performance guides
• Conform by the style guides
• Branch has no merge conflicts with master (if it does - rebase it please)
• Squashed related commits together

What are the relevant issue numbers?

Closes #30237 (closed)

@grzesiek @ayufan I need your help

Server is returning 500:

This is the response:

LoadError at /root/review-app/pipelines/115/retry.json
======================================================

> Unable to autoload constant Pipeline, expected /gitlab-development-kit/gitlab/app/models/ci/pipeline.rb to define it

lib/gitlab/middleware/multipart.rb, line 93
-------------------------------------------

ruby
   88           @app = app
   89         end
   90   
   91         def call(env)
   92           encoded_message = env.delete(RACK_ENV_KEY)
>  93           return @app.call(env) if encoded_message.blank?
   94   
   95           message = Gitlab::Workhorse.decode_jwt(encoded_message)[0]
   96   
   97           Handler.new(env, message).with_open_files do
   98             @app.call(env)

Response headers do not match request headers but I do not know how to fix this. Can you please help me?

(We need to do the same for environments)

Thank you

added ~12980 label

changed milestone to %9.2

assigned to @filipa

@filipa Are you using Postman? I've seen this problem already, but is quite unexpected given the changes in this MR.

By the way, it seems that we have related specs failure https://gitlab.com/gitlab-org/gitlab-ce/issues/25737.

@grzesiek no, I am not using Postman, I changed the controller, stopped gdk and run it again, and I get this error.

If I call it without .json it works any ideas?

@grzesiek can you please take over the backend part of this? Going to assign to you please assign back to me if you can't work on this one

assigned to @grzesiek

@filipa I pushed a commit with following changes.

diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 0e027b1417..2afec5bf7d 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -97,9 +97,7 @@ class Projects::PipelinesController < Projects::ApplicationController
         redirect_back_or_default default: namespace_project_pipelines_path(project.namespace, project)
       end
 
-      format.json do
-        render status: 204
-      end
+      format.json { head :no_content }
     end
   end
 
@@ -111,9 +109,7 @@ class Projects::PipelinesController < Projects::ApplicationController
         redirect_back_or_default default: namespace_project_pipelines_path(project.namespace, project)
       end
 
-      format.json do
-        render status: 204
-      end
+      format.json { head :no_content }
     end
   end

It should resolve exceptions, you probably encountered, related to missing template. Let me know if this works for you!

assigned to @filipa

added 1 commit

• a2ca5b07 - Respond with no content for pipeline JSON actions

Compare with previous version

Thank you @grzesiek

ran it locally and still seeing 500 in both retry and cancel :

LoadError at /root/ci-mock/pipelines/121/cancel.json
====================================================

> Unable to autoload constant Pipeline, expected /gitlab-development-kit/gitlab/app/models/ci/pipeline.rb to define it

lib/gitlab/middleware/multipart.rb, line 93
-------------------------------------------

 ruby
   88           @app = app
   89         end
   90   
   91         def call(env)
   92           encoded_message = env.delete(RACK_ENV_KEY)
>  93           return @app.call(env) if encoded_message.blank?
   94   
   95           message = Gitlab::Workhorse.decode_jwt(encoded_message)[0]
   96   
   97           Handler.new(env, message).with_open_files do
   98             @app.call(env)

What am I doing wrong? /cc @ayufan @grzesiek

added ~901060 label

@filipa How do you get this error? How do you invoke POST to cancel.json? Is this what you get from the Vue component?

@grzesiek we invoke the post request here: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10584/diffs#ab4d46b7ee4a8ae2cd08043ff470c5926d8931f2_44_43

exactly like we were doing before, when the user clicks the button a post request is made.

@grzesiek Only changed 2 things in the frontend:

1. Content-type is now JSON
2. Endpoint has .json in the end

mentioned in merge request !10777 (merged)

@ayufan can you please help me with this one? Thanks!

assigned to @ayufan

@filipa I've seen this error before. It appears that this is not related to changes in this MR. I suppose this might be a GDK problem. I would need to reproduce it locally to investigate this. What code should I paste into javascript console to reproduce this? I'm not able to reproduce this using httpie or curl or a web browser.

assigned to @grzesiek

I'll start a new GDK and see if I have the same problem there. It's possible that I've broken mine

added 653 commits

• a2ca5b07...e9f50438 - 652 commits from branch master
• 701c4719 - Merge branch 'master' into 30237-pipelines-actions-make-2-requests

Compare with previous version

added 3 commits

• 052aea54 - Retry and cancel endpoints send 204 json response. Request is made with type json
• 2cc5530f - Respond with no content for pipeline JSON actions
• 23a38688 - Use wrap_parameters in pipelines controller

Compare with previous version

Thank you so much @grzesiek tested locally and works perfectly.

assigned to @filipa

This has no more frontend work to be done. @iamphill can you check the Javascript part? With this change we don't need to modify the content-type anymore since it will accept JSON

@grzesiek I'll assign this one to @ayufan to review backend

assigned to @ayufan

@filipa Frontend change looks fine to me Happy to get rid of the emulateJSON option

@ayufan will this get in for %9.2 ?

mentioned in issue #31841 (closed)

added 789 commits

• 23a38688...3441e60f - 785 commits from branch master
• 7473c5dc - Retry and cancel endpoints send 204 json response. Request is made with type json
• d6bbc1a7 - Respond with no content for pipeline JSON actions
• ff0c81ac - Use wrap_parameters in pipelines controller
• ff69aa2b - Add specs for new pipeline action for JSON format

Compare with previous version

added 35 commits

• ff69aa2b...3a2b60f7 - 31 commits from branch master
• a1debf5c - Retry and cancel endpoints send 204 json response. Request is made with type json
• b8960354 - Respond with no content for pipeline JSON actions
• c68bf432 - Use wrap_parameters in pipelines controller
• 4f3dc19a - Add specs for new pipeline action for JSON format

Compare with previous version

resolved all discussions

approved this merge request

merged

mentioned in commit a5347fe5