Show the status code for an internal API error without a message

What does this MR do and why?

Closes #883 (closed)

parseError in client/gitlabnet.go falls back to Internal API error (<status>) only when an error body does not decode as JSON (L181). A body that decodes but has no message, such as {"error":"404 Not Found"} (the shape Grape gives a path the API does not have or a parameter validation error), {} or null, gave an APIError with an empty Msg. gitlab-shell then wrote nothing to the SSH client, since console.DisplayWarningMessage skips a blank message, and gitlab-sshd wrote ERROR: with nothing after it. The issue has the details and a reproduction with the binary.

This MR treats an empty message like a body that does not decode. That is the condition ParseJSON in internal/clients/gitlab/response.go already uses (L26), so both parsers now give the same message for the same response. The new table test TestParseErrorMessage pins the message for each body shape.

Behavior change: an error response whose JSON body has no message, or an empty one, now gives Internal API error (<status>) instead of an empty error. Both branches set StatusCode from the response and the System classification with IsSystemErrorStatus, so neither changes. A response with a non-empty message is unchanged.

How to set up and validate locally

  1. Run the new test:

    go test ./client -run TestParseErrorMessage -count=1 -v

    With the test on main's gitlabnet.go, the four cases with a JSON body and no message fail with actual : "", and the other three pass, since they pin what main already does. With the change all seven pass.

  2. To see it in the binary, follow the steps in the issue. On main gitlab-shell exits with 1 and prints nothing; with this change it writes Internal API error (404) in the warning block.

What I ran

In registry.gitlab.com/gitlab-org/gitlab-build-images/debian-trixie-golang-1.25:latest (go1.25.14), the image the tests and lint jobs use, after the test job's make build and cp config.yml.example config.yml, without the Gitaly service (so the gitlab-sshd acceptance tests skip):

  • make verify, make lint (golangci-lint 2.12.2, no difference from the empty support/lint_last_known_acceptable.txt), go vet ./..., go mod tidy (no diff) and make validate-log-fields: pass.
  • make test, make test_race and CGO_ENABLED=0 go test -count 1 ./...: pass, no failing package.
  • nilaway at the version .gitlab-ci.yml pins: no findings.

With Go 1.26.9 and Go 1.27.3: go test -race -count=1 on ./client/..., ./internal/gitlabnet/..., ./internal/clients/... and ./internal/command/...: pass.

Without the change, with Go 1.25.10 (the go.mod version):

--- FAIL: TestParseErrorMessage (0.00s)
    --- PASS: TestParseErrorMessage/a_structured_message_is_passed_through (0.00s)
    --- PASS: TestParseErrorMessage/a_non-JSON_body_falls_back_to_the_status_code (0.00s)
    --- PASS: TestParseErrorMessage/a_JSON_array_falls_back_to_the_status_code (0.00s)
    --- FAIL: TestParseErrorMessage/a_JSON_object_without_a_message_falls_back_to_the_status_code (0.00s)
    --- FAIL: TestParseErrorMessage/an_empty_JSON_object_falls_back_to_the_status_code (0.00s)
    --- FAIL: TestParseErrorMessage/an_empty_message_falls_back_to_the_status_code (0.00s)
    --- FAIL: TestParseErrorMessage/a_JSON_null_falls_back_to_the_status_code (0.00s)

Each failing case reports the fallback it expects (for example expected: "Internal API error (503)") and actual : "".

With the change:

--- PASS: TestParseErrorMessage (0.00s)
    --- PASS: TestParseErrorMessage/a_structured_message_is_passed_through (0.00s)
    --- PASS: TestParseErrorMessage/a_non-JSON_body_falls_back_to_the_status_code (0.00s)
    --- PASS: TestParseErrorMessage/a_JSON_array_falls_back_to_the_status_code (0.00s)
    --- PASS: TestParseErrorMessage/a_JSON_object_without_a_message_falls_back_to_the_status_code (0.00s)
    --- PASS: TestParseErrorMessage/an_empty_JSON_object_falls_back_to_the_status_code (0.00s)
    --- PASS: TestParseErrorMessage/an_empty_message_falls_back_to_the_status_code (0.00s)
    --- PASS: TestParseErrorMessage/a_JSON_null_falls_back_to_the_status_code (0.00s)
ok  	gitlab.com/gitlab-org/gitlab-shell/v14/client	0.014s

The binary, built from each tree and started the way sshd's forced command starts it (gitlab-shell key-1 with SSH_ORIGINAL_COMMAND set), against a stub internal API that answers POST /api/v4/internal/allowed with the status and body shown:

Internal API answer main this MR
404, {"error":"404 Not Found"} no output, exit 1 Internal API error (404), exit 1
400, {"error":"key_id is missing"} no output, exit 1 Internal API error (400), exit 1
403, {"message":"The project you were looking for could not be found."} that message, exit 1 that message, exit 1

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports

Loading
Loading