Skip to content
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.

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

Fix web impl streaming line breaks with large rows #333

Merged
merged 7 commits into from
Oct 5, 2024

Conversation

slvrtrn
Copy link
Contributor

@slvrtrn slvrtrn commented Sep 30, 2024

Summary

Resolves #205
See also: #171, #204 (there was the same fix, but for Node.js impl only)

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG

@slvrtrn
Copy link
Contributor Author

slvrtrn commented Sep 30, 2024

SUMMARY:
✔ 562 tests completed
ℹ 4 tests skipped
✖ 4 tests failed

FAILED TESTS:
  [Web] SELECT streaming
    should correctly process multiple chunks
      large amount of rows
        ✖ should work with .json()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8252:43)

        ✖ should work with .stream()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8265:43)

      rows that don't fit into a single chunk
        ✖ should work with .json()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8281:43)

        ✖ should work with .stream()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8294:43)

Apparently, POST with a very large body size does not work well with Chrome Runner :/
For now, it might be better to generate the test data on the CH side.

EDIT: this is due to https://fetch.spec.whatwg.org/#http-network-or-cache-fetch

If contentLength is non-null and httpRequest’s keepalive is true, then:
If the sum of contentLength and inflightKeepaliveBytes is greater than 64 kibibytes, then return a network error.

Disabling keep-alive resolves the test failure.

Copy link

sonarcloud bot commented Oct 5, 2024

@slvrtrn slvrtrn merged commit 1c9b28b into main Oct 5, 2024
27 checks passed
@slvrtrn slvrtrn deleted the fix-web-streaming-line-breaks branch October 5, 2024 14:21
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.

Streaming line breaks in web version
2 participants