fix(functions): treat invoke 2xx as success regardless of body shape (VOL-766)

[connorckong]

Summary

• Fixes VOL-766: volcano functions invoke reported a false error on genuine 2xx responses whenever the function handler returned a non-object body (plain text, HTML, empty, or non-object JSON).
• Root cause (validated): InvokeFunction used the OpenAPI-generated InvokeFunctionWithResponse client and only treated the call as success when JSONDefault/JSON200 were populated. Those fields are only filled for JSON object bodies. For plain text / empty / non-JSON content-types, the typed fields stay nil and the caller fell through to error classification — producing messages like HTTP 200: plain text from function or HTTP 204: No Content. For application/json arrays (and other non-object JSON), the failure is even earlier: ParseInvokeFunctionClientResponse fails during json.Unmarshal into FunctionInvocationResponse (map[string]interface{}), so InvokeFunctionWithResponse returns a parse error before status-based success handling runs.
• Fix: InvokeFunction calls the raw (non-WithResponse) client method, classifies success purely by HTTP status (2xx = success), and decodes the body leniently (JSON object as-is; other JSON/plain text under a "body" key; empty body as {}). Non-2xx responses still go through apiError normalization. No API request/response contract or endpoint changes; only internal/api/functions.go and its tests.

Validation (independent review)

• Reproduced the pre-fix bug against main for plain-text 200, JSON-array 200 (unmarshal/parse failure), and empty 204; confirmed the CLI path surfaces Error: failed to invoke function "...": HTTP 200: ....
• Confirmed the fix returns success on those same cases (including via the functions invoke command harness).
• Ran full suite: go build ./..., go vet ./..., go test ./... -count=1 — all passed.
• Regression hunt: non-2xx paths still error with status preserved for structured error / error_description / message, 400/401/403/404/429/503, plain-text and HTML error bodies, non-error JSON objects/arrays, empty bodies, malformed JSON, and 302 redirects. No case found where a real failure is reported as success.

Test plan

• Unit: TestInvokeFunctionSuccessWithNonObjectBody (plain text, no content-type object JSON, JSON array, empty 204)
• Unit: existing TestInvokeFunctionErrorsNormalize (429) still passes
• Full go test ./... -count=1
• Optional smoke: deploy/invoke a function that returns plain text or a JSON array and confirm volcano functions invoke prints success JSON

Made with Cursor

[CLAassistant]

All committers have signed the CLA.

[tkkhq]

Feedback

[nit] "no content type" test case still gets an auto-set Content-Type header — internal/api/client_test.go:659-688 RIGHT
The "no content type" case (internal/api/client_test.go lines 659, 683-688) intends to check decoding a JSON object body when no Content-Type header is set. In practice, Go's net/http server sniffs and auto-sets Content-Type: text/plain; charset=utf-8 once Write is called without an explicit header — verified locally with the same WriteHeader-then-Write pattern used here. The response the client actually receives always carries a Content-Type.

This doesn't hide a real bug since decodeInvocationResponseBody never reads Content-Type at all, so the assertion is still valid coverage of JSON-object decoding — it's a naming/intent mismatch, not a functional gap.

Verdict: Approve — No P0/P1 findings; the fix correctly addresses the reported bug for the documented scenarios, existing error-path behavior and callers are unaffected, and the only surviving issues are a non-blocking memory-growth risk on large non-object bodies and two minor/nit-level test and edge-case observations.

[tkkhq]

[P2] Large non-object invoke bodies get fully expanded into interface{}

In decodeInvocationResponseBody (internal/api/functions.go lines ~151-163), when a 2xx body isn't a JSON object, the code falls back to json.Unmarshal(body, &asValue) with asValue any (lines 159-161). For a large JSON array or deeply nested value, this fully materializes every element as boxed interface{} — memory usage can run several times the raw body size (e.g. a several-MB numeric array can balloon into hundreds of MB).

Before this PR, an array response on this success path errored out inside the generated client's map-typed unmarshal without that expansion, so this memory cost is newly reachable via the fix's own success path and has no size guard. This only bites when a deployed function handler returns a large non-object body, but since function handlers are arbitrary user code, that's a realistic condition worth a size cap or streaming decode rather than an unconditional full-value unmarshal.

[tkkhq]

[P3] Literal JSON null invoke body silently returns a nil map, not {"body":null}

decodeInvocationResponseBody's doc comment (internal/api/functions.go lines 148-150) states any non-object JSON value is wrapped under a "body" key. A bare JSON null body breaks that: json.Unmarshal(body, &asMap) (line 156) succeeds with asMap left nil — null is valid input for a Go map — so the function returns a pointer to a nil FunctionInvocationResponse instead of {"body": null}. This also differs from the explicit empty-body case (line 153), which returns a non-nil empty map. Verified locally: unmarshaling null into map[string]interface{} returns err=nil, map=nil.

Impact is low (no crash; JSON-encodes as null instead of {}/{"body":null}) since a handler literally returning bare null as its whole body is rare, but it's a real, silent deviation from the documented contract worth an explicit null check.

[connorckong]

@tkkhq looks like a CI check here is failing to run. any idea?