Skip to content

Response Streaming in JS#3395

Merged
Cole-Greer merged 2 commits intomasterfrom
JS-streaming
May 2, 2026
Merged

Response Streaming in JS#3395
Cole-Greer merged 2 commits intomasterfrom
JS-streaming

Conversation

@Cole-Greer
Copy link
Copy Markdown
Contributor

@Cole-Greer Cole-Greer commented Apr 21, 2026

Add streaming HTTP response handling to gremlin-javascript, enabling incremental result consumption via fetch response.body ReadableStream.

  • Add StreamReader abstraction for async byte reading over streaming and buffered sources
  • Refactor all ~20 GraphBinary serializers from sync deserialize(buffer) to async deserialize(reader)
  • Refactor GraphBinaryReader.readResponse() to use StreamReader
  • Add Connection.stream() using fetch response.body ReadableStream
  • Add Client.stream() returning AsyncGenerator
  • Wire Traversal API (next(), hasNext(), toList()) to streaming for incremental consumption, matching Go GLV behavior
  • submit() remains non-streaming, buffers full response
  • Remove dead readable-stream dependency and Readable imports

VOTE +1

Comment thread gremlin-js/gremlin-javascript/lib/driver/connection.ts Outdated
@kenhuuu
Copy link
Copy Markdown
Contributor

kenhuuu commented Apr 29, 2026

Is there a test for checking that the GraphBinaryReader handles values that are input across chunk boundaries? This is technically covered by the new tests for the StreamReader, but it isn't quite the integration test that I'm expecting as it doesn't go from test input through the GraphBinary deserializers.

Copy link
Copy Markdown
Contributor

@GumpacG GumpacG left a comment

Choose a reason for hiding this comment

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

Could you also add an integration test that would verify early result consumption before the full response completes?

Otherwise, LGTM. VOTE +1

Comment thread gremlin-js/gremlin-javascript/test/unit/graphbinary/error-cases-test.js Outdated
Add streaming HTTP response handling to gremlin-javascript, enabling
incremental result consumption via fetch response.body ReadableStream.

- Add StreamReader abstraction for async byte reading over streaming
  and buffered sources
- Refactor all ~20 GraphBinary serializers from sync deserialize(buffer)
  to async deserialize(reader)
- Refactor GraphBinaryReader.readResponse() to use StreamReader
- Add Connection.stream() using fetch response.body ReadableStream
- Add Client.stream() returning AsyncGenerator
- Wire Traversal API (next(), hasNext(), toList()) to streaming for
  incremental consumption, matching Go GLV behavior
- submit() remains non-streaming, buffers full response
- Remove dead readable-stream dependency and Readable imports
@Cole-Greer
Copy link
Copy Markdown
Contributor Author

Cole-Greer commented Apr 30, 2026

@kenhuuu

Is there a test for checking that the GraphBinaryReader handles values that are input across chunk boundaries? This is technically covered by the new tests for the StreamReader, but it isn't quite the integration test that I'm expecting as it doesn't go from test input through the GraphBinary deserializers.

I've added some cases at the end of GraphBinaryReader-test.js to give more coverage here. They validate that GraphBinaryReader can handle items split across chunk boundaries, as well as validating it can incrementally produce the first result prior to all data being available in to the stream. It doesn't quite reach the level of a full integration test as it's not wired into DRC. Such a test would be useful but needs dedicated server infrastructure, which I consider out of scope here.

@GumpacG

Could you also add an integration test that would verify early result consumption before the full response completes?

Piggy-backing on my above response, I believe the new unit tests added to GraphBinaryReader are the best we can reasonably do for now, without dedicated server infrastructure to control size and timing of response chunks.

Comment thread gremlin-js/gremlin-javascript/lib/driver/connection.ts
Comment thread docs/src/upgrade/release-4.x.x.asciidoc
@xiazcy
Copy link
Copy Markdown
Contributor

xiazcy commented May 1, 2026

VOTE +1

1 similar comment
@kenhuuu
Copy link
Copy Markdown
Contributor

kenhuuu commented May 1, 2026

VOTE +1

@Cole-Greer Cole-Greer merged commit 62bd6da into master May 2, 2026
71 of 74 checks passed
@Cole-Greer Cole-Greer deleted the JS-streaming branch May 2, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants