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
-
Run the new test:
go test ./client -run TestParseErrorMessage -count=1 -vWith the test on
main'sgitlabnet.go, the four cases with a JSON body and no message fail withactual : "", and the other three pass, since they pin whatmainalready does. With the change all seven pass. -
To see it in the binary, follow the steps in the issue. On
maingitlab-shell exits with 1 and prints nothing; with this change it writesInternal 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 emptysupport/lint_last_known_acceptable.txt),go vet ./...,go mod tidy(no diff) andmake validate-log-fields: pass.make test,make test_raceandCGO_ENABLED=0 go test -count 1 ./...: pass, no failing package.nilawayat the version.gitlab-ci.ymlpins: 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.014sThe 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.