Skip to content

Add Detailed Error Message for Credit Card-less Users

Allison Browne requested to merge 346304-refactor-retry-services into master

What does this MR do and why?

This MR adds an error message for all types of json api's when access is denied to retrying a pipeline due to no credit card on file. There are a few changes to the UI. Mostly that a generic flash error message appears upon retry rather than a silent failure of the retry.

The graphql portion of this is complicated by an existing bug where raising Gitlab::Access::AccessDeniedError returns a 500 from the graphql endpoint instead of a 200. This is not isolated to this mutation. #354023 (comment 859070887).

Since Gitlab::Access::AccessDeniedError is rescued globally in the rails api and application controller we have many services that raise Gitlab::Access::AccessDeniedError and expect the application to serve a 403. Instead when these are called in graphql mutations we get a 500 currently { errors: [{message: 'Internal Server Error: GitLab::Access::AccessDeniedError'}] }. This is a bug condidion since we should not throw a 500 in a mutation if access is denied but I think it should be a 200 with 'not found' or another custom error message in the case of a mutation. In a resolver we should return null and 200).

Some examples:

Mutations::Ci::Job::Play calls ::Ci::PlayBuildService which raises Gitlab::Access::AccessDeniedError

Mutations::Ci::Job::Retry calls Ci::RetryPipelineService calls Ci::RetryBuildService both of which can raise Gitlab::Access::AccessDeniedError

We raise this is 48 services currently(some currently called in mutations and many with the potential to be called in mutations).

See the existing conditions and new conditions below:

Screenshots or screen recordings

Existing Conditions (Access Denied)

api type status code response UI desc UI screenshot
graphql 500 { errors: [{message: 'Internal Server Error: GitLab::Access::AccessDeniedError'}] } generic flash error Screen_Shot_2022-02-16_at_1.20.37_PM
Rails controller json 404 or 403 (404 if before_action hit, 403 if AccessDeniedError raised) Screen_Shot_2022-02-16_at_1.48.40_PM
Rails API 404 or 403(404 if before_action hit, 403 if AccessDeniedError raised) {"message": "403 Forbidden"}

Changes (Access Denied)

api type status code response UI desc UI screenshot
graphql 200 {"data":{"pipelineRetry":{"errors":["Gitlab::Access::AccessDeniedError"],"__typename":"PipelineRetryPayload"}}} generic flash error Screen_Shot_2022-02-16_at_2.39.26_PM
Rails controller json 404/403 {"errors":["403 Forbidden"]} generic flash error Screen_Shot_2022-02-16_at_2.26.59_PM
Rails API 404/403 {"message": "403 Forbidden"}

Changes (Access Denied - Credit Card)

api type status code response UI desc UI screenshot
graphql 200 {"data":{"pipelineRetry":{"errors":["Credit card required to be on file in order to retry a pipeline"],"__typename":"PipelineRetryPayload"}}} generic flash error Screen_Shot_2022-02-16_at_2.39.26_PM
Rails controller json 404/403 {"errors":["Credit card required to be on file in order to retry a pipeline"]} generic flash error Screen_Shot_2022-02-16_at_2.26.59_PM
Rails API 404/403 {"message": "Credit card required to be on file in order to retry a pipeline"}

How to set up and validate locally

Example below:

  1. Set has_required_credit_card_to_run_pipelines? to false
     def has_required_credit_card_to_run_pipelines?(project)
       false #has_valid_credit_card? || !requires_credit_card_to_run_pipelines?(project)
     end
  2. Visit any group or project member pages such as http://127.0.0.1:3000/groups/flightjs/-/group_members
  3. Run a failing pipeline. ex script: exit 0
  4. Click the Pipelines button.
  5. Click the cirular 'retry' arrow
  6. Click the pipeline 'retry' arrow
  7. Navigate to the pipeline show page
  8. Click the 'retry' button on
  9. Use curl or httpie to run an http request against your pipeline:
$ http POST "http://localhost:3000/api/v4/projects/10/pipelines/39/retry" "PRIVATE-TOKEN: ***"
HTTP/1.1 403 Forbidden
Cache-Control: no-cache
Content-Length: 77
Content-Type: application/json
Date: Thu, 17 Feb 2022 15:05:29 GMT
Vary: Origin
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Request-Id: 01FW43EY2YGE8K1RR1HSE6AJ8A
X-Runtime: 1.985245

{
    "message": "Credit card required to be on file in order to retry a pipeline"
}

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #346304 (closed)

Edited by Allison Browne

Merge request reports