Warn when GitLab refuses a DELETE

What does this MR do and why?

A rule with delete: true reports nothing when GitLab refuses the DELETE. HttpartyAdapter#delete raises only on 401, 429 and 500, and for any other status returns the body and the rate-limit values without the status, and RestAPINetwork#delete_api uses what it gets only for the rate-limit debug line and wait. A DELETE answered with 403 (a token that may not delete the resource), 404 (a URL that names the wrong project, as the group-source DELETEs did before !419 (merged)) or 502 therefore prints nothing about the answer, not even with --debug, and the run reads like one that deleted the resource.

As suggested in the review of !419 (merged), HttpartyAdapter#delete now also returns the status, and RestAPINetwork#delete_api prints a warning with Gitlab::Triage::UI.warn when the status is not 2xx:

[WARNING] DELETE https://gitlab.com/api/v4/projects/123/issues/1? failed with status 403: 403 Forbidden

The message is the message field of GitLab's JSON body, or its error field, which GitLab uses for a route it does not know or a parameter it rejects. A body that is not JSON, such as a proxy's HTML page for a 502, gives the status alone. A 204, any other 2xx and a response without a status stay silent. The warning does not stop the run: the rule goes on with its next resource, and 401, 429 and 500 are handled as before. The URL keeps the trailing ? the URL builder adds, as the dry run and the delete_url: debug line print it. One sentence in the README's Delete action section says what a refused deletion prints.

POST, PUT and PATCH have the same gap: post_api and its siblings return GitLab's error body as they return any other JSON object. I left them out of this MR, which is about the delete action.

Tests:

  • spec/gitlab/triage/network_adapters/httparty_adapter_spec.rb: #delete returns the status of a 204, and the status and the body of a 403.
  • spec/gitlab/triage/rest_api_network_spec.rb: #delete_api warns about a 403 and a 404 with GitLab's message, a 404 with its error and a 502 with an HTML page, and returns; it prints nothing for a 204, a 200 and a response without a status.
  • spec/integration/delete_issues_spec.rb: the engine runs a rule over two issues, GitLab answers the first DELETE with 403 and the second with 204, and the run prints the warning for the first and still deletes the second.
The new specs without the fix, and the checks with it

On master at 36c92238, with master's httparty_adapter.rb and rest_api_network.rb, bundle exec rspec spec/gitlab/triage/network_adapters/httparty_adapter_spec.rb spec/gitlab/triage/rest_api_network_spec.rb spec/integration/delete_issues_spec.rb on Ruby 3.3.12:

expected {:ratelimit_remaining => Infinity, :ratelimit_reset_at => nil, :results => nil} to include {:status => 204}
expected {:ratelimit_remaining => Infinity, :ratelimit_reset_at => nil, :results => {"message" => "403 Forbidden"}} to include {:status => 403}
expected block to output "[WARNING] DELETE https://gitlab.com/v4/api/projects/project/issues failed with status 403: 403 Forbidden\n" to stdout, but output nothing
expected block to output "[WARNING] DELETE https://gitlab.com/v4/api/projects/project/issues failed with status 404: 404 Project Not Found\n" to stdout, but output nothing
expected block to output "[WARNING] DELETE https://gitlab.com/v4/api/projects/project/issues failed with status 404: 404 Not Found\n" to stdout, but output nothing
expected block to output "[WARNING] DELETE https://gitlab.com/v4/api/projects/project/issues failed with status 502\n" to stdout, but output nothing
expected block to output /^\[WARNING\] DELETE https:\/\/gitlab\.com\/api\/v4\/projects\/123\/issues\/1\? failed with status 403: 403 Forbidden$/ to stdout, but output "[...]* Total after limiting: 2 resources\n\n\n"
74 examples, 7 failures

The seven failures are the seven new examples that expect a status or a warning; in the integration example, nothing is printed after the resources are counted. The three new examples that expect silence pass, as they should.

With the fix, on the same master:

  • bundle exec rake spec: 1159 examples, 0 failures on Ruby 3.0.7 and 3.3.12.
  • bundle exec rubocop --parallel: 221 files inspected, no offenses detected.

Checklist

Closes #413 (closed)

Merge request reports

Loading
Loading