Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
connorckong wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions internal/api/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,68 @@ func TestInvokeFunctionErrorsNormalize(t *testing.T) {
require.ErrorContains(t, err, "HTTP 429: rate limited")
}

func TestInvokeFunctionSuccessWithNonObjectBody(t *testing.T) {
functionID := mustProjectID(t, "22222222-2222-4222-8222-222222222222")

testCases := []struct {
name string
contentType string
status int
body string
want map[string]any
}{
{
name: "plain text body",
contentType: "text/plain; charset=utf-8",
status: http.StatusOK,
body: "plain text ok",
want: map[string]any{"body": "plain text ok"},
},
{
name: "no content type",
contentType: "",
status: http.StatusOK,
body: `{"ok":true}`,
want: map[string]any{"ok": true},
},
{
name: "json array body",
contentType: "application/json",
status: http.StatusOK,
body: `["a","b"]`,
want: map[string]any{"body": []any{"a", "b"}},
},
{
name: "empty body",
contentType: "",
status: http.StatusNoContent,
body: "",
want: map[string]any{},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
if tc.contentType != "" {
w.Header().Set("Content-Type", tc.contentType)
}
w.WriteHeader(tc.status)
_, _ = w.Write([]byte(tc.body))
}))
defer server.Close()

client, err := NewClient(server.URL, "", WithHTTPClient(server.Client()))
require.NoError(t, err)

resp, err := client.InvokeFunction(context.Background(), functionID, FunctionInvokeInput{})
require.NoError(t, err, "a genuine 2xx response must not be reported as an error")
require.NotNil(t, resp)
assert.Equal(t, tc.want, map[string]any(*resp))
})
}
}

func mustProjectID(t *testing.T, value string) uuid.UUID {
t.Helper()
id, err := uuid.Parse(value)
Expand Down
42 changes: 38 additions & 4 deletions internal/api/functions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"mime/multipart"

"github.com/google/uuid"
Expand Down Expand Up @@ -112,21 +113,54 @@ func (c *Client) UpdateFunctionVisibility(ctx context.Context, projectID, functi
}

// InvokeFunction invokes one function by ID.
//
// The invoke endpoint passes through whatever the target function handler
// returns, so unlike other endpoints its response body isn't guaranteed to
// be a JSON object matching the generated schema (it may be plain text,
// HTML, empty, or non-object JSON). The generated WithResponse client only
// populates JSON200/JSONDefault when the body is a JSON object, so this
// calls the raw client method and classifies success/failure on the HTTP
// status code directly, decoding the body leniently rather than erroring
// out on a genuine 2xx.
func (c *Client) InvokeFunction(ctx context.Context, functionID uuid.UUID, input FunctionInvokeInput) (*apiclient.FunctionInvocationResponse, error) {
body := apiclient.InvokeFunctionJSONRequestBody{}
if input.Payload != nil {
payload := input.Payload
body.Payload = &payload
}

resp, err := c.client.InvokeFunctionWithResponse(ctx, functionID, body)
httpResp, err := c.client.InvokeFunction(ctx, functionID, body)
if err != nil {
return nil, err
}
if resp.JSONDefault != nil && resp.StatusCode() >= 200 && resp.StatusCode() < 300 {
return resp.JSONDefault, nil
defer func() { _ = httpResp.Body.Close() }()
respBody, err := io.ReadAll(httpResp.Body)
if err != nil {
return nil, err
}

if httpResp.StatusCode >= 200 && httpResp.StatusCode < 300 {
return decodeInvocationResponseBody(respBody), nil
}
return apiResult(resp.StatusCode(), resp.Body, resp.JSON200, resp.JSON400, resp.JSON401, resp.JSON403, resp.JSON404, resp.JSON429, resp.JSON503)
return nil, apiError(httpResp.StatusCode, respBody)
}

// decodeInvocationResponseBody interprets a successful invocation's raw body
// leniently: a JSON object is used as-is, any other JSON value or plain-text
// body is wrapped under a "body" key, and an empty body yields an empty map.
func decodeInvocationResponseBody(body []byte) *apiclient.FunctionInvocationResponse {
if len(body) == 0 {
return &apiclient.FunctionInvocationResponse{}
}
var asMap apiclient.FunctionInvocationResponse
if json.Unmarshal(body, &asMap) == nil {
return &asMap
Comment on lines +155 to +157

@tkkhq tkkhq Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

}
var asValue any
if json.Unmarshal(body, &asValue) == nil {
return &apiclient.FunctionInvocationResponse{"body": asValue}
Comment on lines +159 to +161

@tkkhq tkkhq Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

}
return &apiclient.FunctionInvocationResponse{"body": string(body)}
}

// ListFunctionRuntimes returns the function runtime catalog.
Expand Down