Skip to content

Refactor gitlab api response handling

Tomasz Maczukin requested to merge refactor-gitlab-api-response-handling into master

What does this MR do?

Highly refactors response handling in network/gitlab.go.

Why was this MR needed?

During last two days I've spent several hours investigating a regression that was stopping a security release on staging (here and here). The regression was affecting and was found by a problem with artifacts upload. All artifacts uploads on staging.gitlab.com.

A similar, but totally unrelated issue affecting artifacts upload, was reported by one of the customers who is using GitLab 12.9.

In the first problem we've finally found what's causing the upload failures after I've prepared a patched Runner that handles and prints the error message sent by GitLab in response (as JSON) at failures. In the second problem we don't know yet why exactly it happens - exactly because we don't consume the message, we know only that there is a 400 Bad Request response. Which in case of artifacts upload can be caused by several reasons.

It's 3rd or 4th time when the fact that we don't log the error message sent by GitLab for API requests causes huge pain with debugging issue reports. So I've finally decided to add it permanently 🙂.

But to make it complete and working the same for all requests, a certain level of refactoring was required. Including how we pass the responses from the internal client.

Are there points in the code the reviewer needs to double check?

Does this MR meet the acceptance criteria?

  • Documentation created/updated
  • Added tests for this feature/bug
  • In case of conflicts with master - branch was rebased

What are the relevant issue numbers?

Merge request reports