Skip to content

Investigate eval response serialization hazards #61

@dchoi-viant

Description

@dchoi-viant

Written using Cursor and GPT-5.4.

Summary

During investigation of a downstream client error (failed to unmarshal: ''), code inspection did not identify a normal eval success-path branch that intentionally returns 200 OK with an empty body. However, it did uncover two implementation hazards worth documenting and investigating.

Findings

  1. service/transformOutput() starts an asynchronous datastore write before the HTTP response is serialized.

    • go s.datastore.Put(ctx, key, transformed, dictHash) is launched before response.Data is marshaled in the HTTP handler.
    • This means the same transformed object may be traversed concurrently by the background datastore path and the response encoder.
    • This is not a confirmed bug, but it is a real shared-object concurrency hazard.
  2. service/response.go uses enc.EncodeEmbeddedJSON() inside Response.MarshalJSONObject() when response.Data falls back to json.Marshal(...).

    • In vendored gojay, EncodeEmbeddedJSON() is a streaming method that writes via enc.Write(), which assumes an encoder backed by a writer.
    • gojay.Marshal(appResponse) uses a borrowed encoder without a writer, so this fallback path appears unsafe.
    • This does not look like a realistic source of a clean 200 OK + empty body; it is more likely to result in incorrect behavior or panic if exercised.

Why this matters

The current code does not strongly support the theory that mly intentionally emits an empty successful eval response, but the two hazards above weaken confidence in the response serialization path and should be reviewed.

Suggested follow-up

  • Verify whether model output types used in production ever hit the json.Marshal(...) fallback in Response.MarshalJSONObject().
  • Determine whether transformed values returned by transformOutput() are immutable/read-only during response encoding.
  • Consider eliminating the async datastore write or deep-copying the stored value before launching the goroutine.
  • Consider replacing EncodeEmbeddedJSON() with a non-streaming embedded JSON append path during gojay.Marshal(...) object construction.

Scope note

This issue documents suspicious behavior found during code inspection. It does not claim a confirmed root cause for the downstream empty-body decode failure.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions