From e505923479e7bc806e2966c69277c0307c574497 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Thu, 28 May 2026 20:12:08 +0000 Subject: [PATCH] graphql: emit wire request + raw response under --http.log.enabled; fix typos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #103. The GraphQL acquire path's --http.log.enabled output only surfaced the post-transform projection, hiding the rendered query string and the naked pre-transform response. Both are essential for diagnosing template-rendering and response-transform failures. Add an exported ContextWithHTTPLogger(ctx, io.Writer) so callers (stackql) can attach the same writer they hand to the REST acquire path when runtimeCtx.HTTPLogEnabled is true. Read() now emits the wire URL + request body before Do(), and the raw response bytes before json.Decode — matching the REST line format so existing log tooling keeps working. With no logger in the context, behaviour is byte-identical to today. Also fix two long-standing typos in the same Read() function's error messages: "accomodate" -> "accommodate" and "pocessed" -> "processed". A package-wide grep for the typo set listed in the issue surfaced only these two sites. Co-authored-by: Jeffrey Aven Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/graphql/graphql.go | 50 ++++++++++++++++++++++- pkg/graphql/graphql_test.go | 79 +++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 2 deletions(-) diff --git a/pkg/graphql/graphql.go b/pkg/graphql/graphql.go index 24f3fcb..a5cd529 100644 --- a/pkg/graphql/graphql.go +++ b/pkg/graphql/graphql.go @@ -2,6 +2,7 @@ package graphql import ( "bytes" + "context" "encoding/json" "fmt" "io" @@ -15,6 +16,36 @@ import ( "github.com/stackql/any-sdk/pkg/stream_transform" ) +// httpLoggerCtxKey is the context key under which an optional io.Writer is +// attached so the GraphQL reader can emit the wire request body and the raw +// pre-transform response. Mirrors the REST acquire path which writes the same +// shape of lines to runtimeCtx.outErrFile when --http.log.enabled is set. +type httpLoggerCtxKey struct{} + +// ContextWithHTTPLogger returns a derived context that carries w as the sink +// for GraphQL wire request / raw response log lines. Consumers (e.g. stackql) +// should attach the same writer they use for REST HTTP logging when +// --http.log.enabled is true. Passing a nil writer is equivalent to not +// attaching one. +func ContextWithHTTPLogger(ctx context.Context, w io.Writer) context.Context { + if w == nil { + return ctx + } + return context.WithValue(ctx, httpLoggerCtxKey{}, w) +} + +func httpLoggerFromContext(ctx context.Context) io.Writer { + if ctx == nil { + return nil + } + v := ctx.Value(httpLoggerCtxKey{}) + if v == nil { + return nil + } + w, _ := v.(io.Writer) + return w +} + var ( _ template.ExecError = template.ExecError{} ) @@ -298,6 +329,14 @@ func (gq *StandardGQLReader) Read() ([]map[string]interface{}, error) { req.Body = rb req.URL.RawQuery = "" req.Header.Set("Content-Type", "application/json") + if logger := httpLoggerFromContext(req.Context()); logger != nil { + bodyBytes, readErr := io.ReadAll(req.Body) + if readErr == nil { + req.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + fmt.Fprintf(logger, "http request url: '%s', method: '%s'\n", req.URL.String(), req.Method) + fmt.Fprintf(logger, "http request body = '%s'\n", string(bodyBytes)) + } + } r, err := gq.anySdkClient.Do( newAnySdkGraphQLHTTPDesignation(req.URL), newGraphqlAnySdkArgList(newAnySdkHTTPArg(req)), @@ -309,6 +348,13 @@ func (gq *StandardGQLReader) Read() ([]map[string]interface{}, error) { if httpResponseErr != nil { return nil, httpResponseErr } + if logger := httpLoggerFromContext(req.Context()); logger != nil && httpResponse != nil && httpResponse.Body != nil { + respBytes, readErr := io.ReadAll(httpResponse.Body) + if readErr == nil { + httpResponse.Body = io.NopCloser(bytes.NewReader(respBytes)) + fmt.Fprintf(logger, "%s\n", string(respBytes)) + } + } gq.pageCount++ var target map[string]interface{} err = json.NewDecoder(httpResponse.Body).Decode(&target) @@ -340,11 +386,11 @@ func (gq *StandardGQLReader) Read() ([]map[string]interface{}, error) { case map[string]interface{}: rv = append(rv, v) default: - return nil, fmt.Errorf("cannot accomodate GraphQL pocessed response item of type = '%T'", v) + return nil, fmt.Errorf("cannot accommodate GraphQL processed response item of type = '%T'", v) } } default: - return nil, fmt.Errorf("cannot accomodate GraphQL pocessed response of type = '%T'", pr) + return nil, fmt.Errorf("cannot accommodate GraphQL processed response of type = '%T'", pr) } gq.rowsReturned += len(rv) if returnErr == nil { diff --git a/pkg/graphql/graphql_test.go b/pkg/graphql/graphql_test.go index 25fcbf1..7f49e37 100644 --- a/pkg/graphql/graphql_test.go +++ b/pkg/graphql/graphql_test.go @@ -2,6 +2,7 @@ package graphql import ( "bytes" + "context" "io" "net/http" "net/url" @@ -590,6 +591,84 @@ func TestRead_GraphQLErrorWithoutMessage_FallsBackToJSON(t *testing.T) { } } +// TestRead_EmitsRequestBodyToHTTPLogWhenEnabled asserts that when a context +// logger is attached, the rendered GraphQL request body and wire URL are +// surfaced before the Do() call — closing the gap where --http.log.enabled +// previously only showed the post-transform projection. +func TestRead_EmitsRequestBodyToHTTPLogWhenEnabled(t *testing.T) { + var buf bytes.Buffer + c := &fakeAnySdkClient{bodyJSON: `{"data": {"rows": [{"id": 1}]}}`} + req := newTestRequest(t) + req = req.WithContext(ContextWithHTTPLogger(context.Background(), &buf)) + + r, err := NewStandardGQLReader( + c, req, 0, `query { rows { id } }`, map[string]interface{}{}, "", + "$.data.rows[*]", "$.data.__no_cursor[*]", + ) + if err != nil { + t.Fatalf("NewStandardGQLReader: %v", err) + } + if _, err := r.Read(); err != nil && err != io.EOF { + t.Fatalf("Read: %v", err) + } + + out := buf.String() + if !strings.Contains(out, "query { rows { id } }") { + t.Errorf("expected rendered request body in log, got:\n%s", out) + } + if !strings.Contains(out, "https://api.example.test/graphql") { + t.Errorf("expected wire URL in log, got:\n%s", out) + } +} + +// TestRead_EmitsRawResponseToHTTPLogWhenEnabled asserts that the naked +// pre-transform response body is surfaced when a context logger is attached. +// This is the diagnostic that was missing for transform / templating failures. +func TestRead_EmitsRawResponseToHTTPLogWhenEnabled(t *testing.T) { + var buf bytes.Buffer + c := &fakeAnySdkClient{bodyJSON: `{"data":{"rows":[{"id":1}]}}`} + req := newTestRequest(t) + req = req.WithContext(ContextWithHTTPLogger(context.Background(), &buf)) + + r, err := NewStandardGQLReader( + c, req, 0, `{ ignored }`, map[string]interface{}{}, "", + "$.data.rows[*]", "$.data.__no_cursor[*]", + ) + if err != nil { + t.Fatalf("NewStandardGQLReader: %v", err) + } + if _, err := r.Read(); err != nil && err != io.EOF { + t.Fatalf("Read: %v", err) + } + + out := buf.String() + if !strings.Contains(out, `"id":1`) { + t.Errorf("expected raw response body in log, got:\n%s", out) + } +} + +// TestRead_DoesNotLogWhenHTTPLogDisabled asserts that with no logger attached +// to the request context, Read() emits nothing — the opt-in shape mirrors the +// REST acquire path's gating on runtimeCtx.HTTPLogEnabled. +func TestRead_DoesNotLogWhenHTTPLogDisabled(t *testing.T) { + c := &fakeAnySdkClient{bodyJSON: `{"data":{"rows":[]}}`} + req := newTestRequest(t) // no logger in context + + r, err := NewStandardGQLReader( + c, req, 0, `{ ignored }`, map[string]interface{}{}, "", + "$.data.rows[*]", "$.data.__no_cursor[*]", + ) + if err != nil { + t.Fatalf("NewStandardGQLReader: %v", err) + } + if _, err := r.Read(); err != nil && err != io.EOF { + t.Fatalf("Read: %v", err) + } + // nothing to assert beyond "no panic and no log sink to fill" — the + // negative case is covered by the structural check that nil-logger + // branches in Read() are short-circuit. +} + // TestNewStandardGQLReaderWithCursor_KeysetRequiresFormat ensures a keyset // configuration without a format template is rejected at construction time. func TestNewStandardGQLReaderWithCursor_KeysetRequiresFormat(t *testing.T) {