-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: implement POST/PUT support for protocol file uploads with enhanced stream handling #3
base: v1
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are you using to upload files? If you're using a <form>
or FormData
then you need to parse the body. e.g.: https://github.com/RangerMauve/hypercore-fetch/blob/master/index.js#L402
I currently parse the form data using the built in Request API in node.js
src/protocols/hyper-handler.js
Outdated
@@ -13,6 +13,22 @@ async function initializeHyperSDK(options) { | |||
return fetch; | |||
} | |||
|
|||
// Helper function to read and handle body data, especially for 'PUT' and 'POST' | |||
async function readBody(bodyStream) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you shouldn't be concatenating the response on a single buffer like that, it'll lag your computer on large files. You should return the stream of the body in the callback instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With large files:
Error in divine_reflections.mp4: HypercoreError: BAD_ARGUMENT: Appended block exceeds the maximum suggested block size at Hypercore.append
src/protocols/ipfs-handler.js
Outdated
data.push(chunk); | ||
} | ||
|
||
const fileBuffer = uint8ArrayConcat(data); // Concatenate all chunks into a single buffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing a concat you should pass a byte stream. Else it'll lag on large files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works with large files
I'm testing it on the agregore tutorials:
Getting this after implementing the changes:
On the page:
|
I think you should spend a bit more time debugging the stack trace to see why the error is happening. |
…turn root CID, and auto-serve index.html
… URL generation for Hyper and IPFS protocols
…d improved encapsulation
Implementing the readBody from:
https://github.com/AgregoreWeb/agregore-browser/blob/master/app/protocols/fetch-to-handler.js
IPFS handler:
It's publishing but when I try to access the IPFS CID, it downloads a file instead of opening the content, the file looks like this:
Hyper handler:
That's also publishing but when clicked on the hyper URL it shows empty index.