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

toBuffer() returns info instead of data, when called after toFile() #3044

Closed
jojomatik opened this issue Jan 16, 2022 · 5 comments
Closed

toBuffer() returns info instead of data, when called after toFile() #3044

jojomatik opened this issue Jan 16, 2022 · 5 comments

Comments

@jojomatik
Copy link

Are you using the latest version? Is the version currently in use as reported by npm ls sharp the same as the latest version as reported by npm view sharp dist-tags.latest?

Yes, 0.29.3.

What are the steps to reproduce?

  1. Install sharp
  2. Load from buffer
    const buffer = Buffer.from(image, "base64");
    const sharpSkin = sharp(buffer);
  3. Apply some modifications to image (propably not strictly necessary)
    const sharpHead = sharpSkin.extract({ left: 8, top: 8, width: 8, height: 8 });
  4. Output to file and to buffer.
    await sharpHead.toFile("test.png");
    return sharpHead
      .toBuffer()
      .then((data) => {
        return data.toString("base64");
      })
      .catch((error) => {
        throw new Error(
          "Couldn't extract head of skin for uuid " + uuid + ". Reason: " + error
        );
      });

What is the expected behaviour?

Either of two options:

  • Return data as expected instead of the info object.
  • Throw error and don't return anything.

Are you able to provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem?

const buffer = Buffer.from(image, "base64");
const sharpSkin = sharp(buffer);
const sharpHead = sharpSkin.extract({ left: 8, top: 8, width: 8, height: 8 });
await sharpHead.toFile("test.png"); // works as expected
return sharpHead
 .toBuffer()
 .then((data) => {
   console.log(JSON.stringify(data)); // `{"format":"png","width":8,"height":8,"channels":4,"premultiplied":false,"size":326}`
   return data.toString("base64"); // returns "[object Object]"
 })
 .catch((error) => {
   throw new Error(
     "Couldn't extract head of skin for uuid " + uuid + ". Reason: " + error
   );
 });

If you explicitly force sharp to output the info object, data still returns info and info is undefined in this case.

Are you able to provide a sample image that helps explain the problem?

From my understanding this happens with every image, therefore not applicable.

What is the output of running npx envinfo --binaries --system?

  System:
    OS: Windows 10 10.0.19043
    CPU: (12) x64 AMD Ryzen 5 3600 6-Core Processor
    Memory: 6.76 GB / 15.93 GB
  Binaries:
    Node: 16.13.0 - G:\Programme\nodejs\node.EXE
    npm: 8.1.0 - G:\Programme\nodejs\npm.CMD
@lovell
Copy link
Owner

lovell commented Jan 17, 2022

Hi, this looks like single input multiple output, which suggests that you might want to clone() the sharp instance before calling toFile() or toBuffer().

https://sharp.pixelplumbing.com/api-constructor#clone

- await sharpHead.toFile("test.png"); // works as expected
+ await sharpHead.clone().toFile("test.png"); // works as expected

Having said that, given there is no Stream based I/O here, we should be able to make an underlying improvement to sharp so the use of file and Buffer based output on the same instance can occur without a need to clone; I'll have a think about this.

@lovell lovell added enhancement and removed triage labels Jan 17, 2022
@jojomatik
Copy link
Author

jojomatik commented Jan 18, 2022

Thanks @lovell for the fast response.

I actually do not need single input multiple output mechanisms in my production code. I encountered this problem whilst testing my code. I initially used toFile() as it has been easier to debug for me and later added the toBuffer() statement and wondered why the output was not working as intended and couldn't find anyone encountering the same problem while searching the web.

Having said that, given there is no Stream based I/O here, we should be able to make an underlying improvement to sharp so the use of file and Buffer based output on the same instance can occur without a need to clone; I'll have a think about this.

Not needing to clone would be the way to go if you ask me, but I don't know if it's possible from a technical point of view. Otherwise an error message would have been really nice and - although I don't know if this is possible - the typescript type definitions should be adjusted, as typescript still assumes that the return value is of type Buffer.

@lovell
Copy link
Owner

lovell commented Feb 3, 2022

Commit 4246602 adds this scenario as a test case, plus the fix. This will be in v0.30.1, thanks for reporting.

@jojomatik
Copy link
Author

Commit 4246602 adds this scenario as a test case, plus the fix. This will be in v0.30.1, thanks for reporting.

Sounds great, always glad to help, even though I don't have the time to dive into the codebase myself :)

Thanks for the timely fix

@lovell lovell added this to the v0.30.1 milestone Feb 4, 2022
@lovell
Copy link
Owner

lovell commented Feb 9, 2022

v0.30.1 now available.

@lovell lovell closed this as completed Feb 9, 2022
Y2zz pushed a commit to Y2zz/sharp that referenced this issue Feb 26, 2022
martinj pushed a commit to aptoma/sharp that referenced this issue Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants