bug: API client methods panic on non-2xx HTTP responses due to missing status code validation

[gyanranjanpanda]

Summary

Six methods across pkg/connectors/microcks_client.go and pkg/connectors/keycloak_client.go do not check the HTTP response status code before parsing the body and performing type assertions. When the server returns a non-2xx response (401, 403, 500, etc.), the CLI panics with a stack trace instead of returning an actionable error.

Affected Methods

Method	File	Panic Trigger
CreateTestResult	microcks_client.go:L387-392	createTestResp["id"].(string) - panics when "id" key is absent
GetTestResult	microcks_client.go:L396-431	No status check; unmarshals into struct - silent zero-value TestResultSummary on error, causing false test failures
GetKeycloakURL	microcks_client.go:L245-253	Three bare assertions: ["enabled"].(bool), ["auth-server-url"].(string), ["realm"].(string)
ConnectAndGetToken	keycloak_client.go:L103-108	openIDResp["access_token"].(string) - panics on 401
GetOIDCConfig	keycloak_client.go:L133-139	openIDResp["authorization_endpoint"].(string) - panics on bad OIDC discovery
ConnectAndGetTokenAndRefreshToken	keycloak_client.go:L180-186	Both ["access_token"] and ["refresh_token"] assertions panic

Root Cause

Every affected method follows this pattern:

resp, err := c.httpClient.Do(req)
// [x] No status code check!
body, _ := io.ReadAll(resp.Body)
if err != nil {
    panic(err.Error())  // Explicit panic instead of error return
}
var someResp map[string]interface{}
if err := json.Unmarshal(body, &someResp); err != nil {
    panic(err)  // Explicit panic instead of error return
}
value := someResp["key"].(string)  // [x] Bare type assertion - panics directly

Important clarification: json.Unmarshal itself does not panic - the code explicitly calls panic(err) when unmarshal fails. The bare type assertions (e.g., .(string), .(bool)) panic directly when the JSON is valid but doesn't contain the expected fields.

Compare this to UploadArtifact and DownloadArtifact in the same file, which correctly guard with if resp.StatusCode != 201. The inconsistency shows this was an oversight rather than a design choice.

GetTestResult - Silent False Failure

GetTestResult (L396) is a subtle variant: it unmarshals into a TestResultSummary struct rather than a map, so it won't panic on bad JSON. However, a non-2xx response body will silently produce a zero-value TestResultSummary{Success: false, InProgress: false}, causing the polling loop in cmd/test.go to exit immediately and report a false test failure. The real problem (network error, auth expiry) is masked.

Test Coverage Gap

Existing tests in microcks_client_test.go only exercise UploadArtifact and DownloadArtifact happy paths. None of the six affected methods have tests covering non-2xx response paths. go test ./pkg/connectors passes because these failure paths are never exercised.

Steps to Reproduce

# Use invalid Keycloak credentials against an auth-enabled server
microcks test \
  --microcksURL=http://microcks.example.com \
  --keycloakClientId=wrong-id \
  --keycloakClientSecret=wrong-secret \
  "PetStore:1.0" "http://endpoint:3000" "HTTP"

Expected: Error: authentication failed (HTTP 401): invalid client credentials
Actual: panic: interface conversion: interface {} is nil, not string

Impact

• CI/CD pipeline crashes with Go stack traces instead of actionable errors - operators cannot distinguish expired tokens from code bugs
• • Token refresh cascades: refreshAuthToken -> redeemRefreshToken -> ConnectAndGetToken all panic when refresh tokens expire
• • • Silent false failures: GetTestResult returns zero-value results on error responses, making tests appear to fail when the actual problem is network/auth
• • • • No caller recourse: panic() usage prevents callers from handling failures gracefully

Proposed Fix

For each affected method:

1. Add if resp.StatusCode >= 400 { return ..., fmt.Errorf("server returned HTTP %d: %s", resp.StatusCode, body) } before JSON parsing
2. 2. Replace panic() calls with proper return ..., fmt.Errorf(...) error returns
3. 3. Use comma-ok type assertions: id, ok := resp["id"].(string) with error returns on !ok

Relationship to Existing Issues

This is not a duplicate of #395 - that issue covers GetKeycloakURL panics on malformed JSON responses specifically. This issue addresses the systemic pattern across all six affected methods, including CreateTestResult (most used in CI/CD) and the GetTestResult silent false failure which is a distinct, previously unreported failure mode.