-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Cannot pipe output to S3 with @aws-sdk/lib-storage. #3313
Comments
Removing |
Please can you add some error handling here: fs.createReadStream('./input.png').pipe(resizer) My best guess would be that something is going wrong on the input side. |
There doesn't seem to be any error on the input stream. I've also tried input streams from both the fs lib and the fastify-multipart library and it's the same issue. |
This version of Node.js is EOL, please can you upgrade to the latest version 18 release and see if you have the same problem. |
This still occurs with v18.7.0 |
Thanks for confirming, I asked as there have been a number of stream-related changes to Node.js throughout versions 17 and 18. Do you only see this problem on Windows? Are you able to test on e.g. Linux? Looking at the source of the https://github.com/aws/aws-sdk-js-v3/blob/main/lib/lib-storage/src/chunks/getChunkStream.ts The next step is to try to replicate the same problem without |
@BlakeB415 we have a similar flow in our app, and we used a
we're using a newer version of the S3 client, and in the function here also, anecdotally, in general the S3 client behaves strangely with streams, and we've found that our two options are to 1.) pass it the file stream directly, and that's it, or 2.) give it a PassThrough and then use the PassThrough as part of another pipe (as shown in this example) hope this helps |
@BlakeB415 Were you able to make any progress with this? |
Removing this line worked for me. https://github.com/lovell/sharp/blob/main/lib/output.js#L1208 |
@BlakeB415 Thanks for the update, please can you try the following change to see if that fixes it also: --- a/lib/constructor.js
+++ b/lib/constructor.js
@@ -168,7 +168,7 @@ const Sharp = function (input, options) {
if (!(this instanceof Sharp)) {
return new Sharp(input, options);
}
- stream.Duplex.call(this);
+ stream.Duplex.call(this, { emitClose: false });
this.options = {
// resize options
topOffsetPre: -1, I think this might relate to nodejs/node#32965 - it looks like we need to notify Node.js that sharp will emit the |
I still get the error with this patch. |
@BlakeB415 Thanks for checking. Here's an alternative approach, which ensures the - this.emit('close');
+ this.on('end', () => this.emit('close')); |
This worked flawlessly! |
Thanks for confirming, updated via commit 70e6bb0 and this will be in v0.31.1. |
v0.31.1 now available |
Possible bug
Is this a possible bug in a feature of sharp, unrelated to installation?
npm install sharp
completes without error.node -e "require('sharp')"
completes without error.Are you using the latest version of sharp?
sharp
as reported bynpm view sharp dist-tags.latest
.If you are using another package which depends on a version of
sharp
that is not the latest, please open an issue against that package instead.What is the output of running
npx envinfo --binaries --system --npmPackages=sharp --npmGlobalPackages=sharp
?What are the steps to reproduce?
Use AWS NodeJS SDK v3 to stream the image output to a bucket, pipe the output to the Upload instance.
What is the expected behaviour?
It successfully uploads to S3 without error.
Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem
This only happens with the AWS SDK v3 + Sharp so it's hard to replicate without it.
Please provide sample image(s) that help explain this problem
N/A
The text was updated successfully, but these errors were encountered: