Skip to content

fix: Ensure streaming HTTP streams the whole notification in a single chunk#1932

Merged
joachimvh merged 1 commit intoCommunitySolidServer:mainfrom
elf-pavlik:fix/streaming-http-chunk
Aug 5, 2024
Merged

fix: Ensure streaming HTTP streams the whole notification in a single chunk#1932
joachimvh merged 1 commit intoCommunitySolidServer:mainfrom
elf-pavlik:fix/streaming-http-chunk

Conversation

@elf-pavlik
Copy link
Contributor

@elf-pavlik elf-pavlik commented Aug 2, 2024

📁 Related issues

✍️ Description

I have been getting notifications where most of the turtle is in one chunk, and the final . is in a separate chunk.
By using readableToString, the HTTP stream is chunked independently of chunks on representation.data.

The requirement to have the full notification in a single chunk is not yet in the spec, but I'm tracking it in:

✅ PR check list

Before this pull request can be merged, a core maintainer will check whether

  • this PR is labeled with the correct semver label
    • semver.patch: Backwards compatible bug fixes.
    • semver.minor: Backwards compatible feature additions.
    • semver.major: Breaking changes. This includes changing interfaces or configuration behaviour.
  • the correct branch is targeted. Patch updates can target main, other changes should target the latest versions/* branch.
  • the RELEASE_NOTES.md document in case of relevant feature or config changes.
  • any relevant documentation was updated to reflect the changes in this PR.

@joachimvh joachimvh merged commit 3dd8602 into CommunitySolidServer:main Aug 5, 2024
@elf-pavlik elf-pavlik deleted the fix/streaming-http-chunk branch August 5, 2024 15:37
@elf-pavlik
Copy link
Contributor Author

Could 7.1.1 be published in the next few days? I'm using the npm package for automated testing and will not have to skip related tests once this fix is available 🐛 🔨

@joachimvh
Copy link
Member

Done!

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.

2 participants