2

If I resize an image twice (using the sharp library), my route returns undefined. All my resized images are showing up. It's like somehow, by calling image.resize and .toBuffer() twice, it's corrupting the res object.

router.post('/submit', upload.single('file'), (req, res) => {
  var image = sharp(req.file.buffer)
  image.resize(1500, 1500, { fit: 'inside' }).toBuffer()
    .then(buffer => fs.writeFile('../image-md.jpg', buffer))
    .then(() => image.resize(500, 500, { fit: 'inside' }).toBuffer())
    .then(buffer => fs.writeFile('../image-sm.jpg', buffer))
    .then(() => res.json({ success: true })) 
    .catch(err => res.status(400).json({ error: err.message }))
})

// res is undefined, and no status is even returned

The weird thing is, if I comment out one of the resizes, everything works just fine:

router.post('/submit', upload.single('file'), (req, res) => {
  var image = sharp(req.file.buffer)
  image.resize(1500, 1500, { fit: 'inside' }).toBuffer()
    .then(buffer => fs.writeFile('../image-md.jpg', buffer))
    // .then(() => image.resize(500, 500, { fit: 'inside' }).toBuffer())
    // .then(buffer => fs.writeFile('../image-sm.jpg', buffer))
    .then(() => res.json({ success: true })) 
    .catch(err => res.status(400).json({ error: err.message }))
})

// res.data is { success: true } and status 200 is returned

Help!

Glenn
  • 4,195
  • 9
  • 33
  • 41
  • 1
    Except on the last `.then` of a chain, you should only ever return Promises from a `.then` (otherwise, combine with the next `.then`) – CertainPerformance Jan 13 '20 at 04:01
  • So should my last ```.then``` be like ```.then(buffer => { fs.writeFile('../image-sm.jpg', buffer) res.json({ success: true }) })``` – Glenn Jan 13 '20 at 04:08

2 Answers2

0

The main problem is here:

.then(() => image.resize(500, 500, { fit: 'inside' }).toBuffer())

.then should only accept a function as a parameter, but you're pass it the result of calling toBuffer on calling image.resize immediately. Using parentheses to make the grouping easier to understand:

.then(
  (() => image.resize(500, 500, { fit: 'inside' }))
    .toBuffer()
)

Not 100% sure about the conventions here, but a reader of the code (like me) would probably except a reference to fs to refer to the built-in fs module, whose writeFile method doesn't return a Promise, so seeing .then or await being used on it could be confusing - you might consider it naming it something like fsPromises, or something similar (just not fs).

It might be easier to read using async/await:

const processImage = async (requestBuffer) => {
  const image = sharp(requestBuffer);
  const bigBuffer = await image.resize(1500, 1500, { fit: 'inside' }).toBuffer();
  await fsPromises.writeFile('../image-md.jpg', bigBuffer);
  const smallBuffer = await image.resize(500, 500, { fit: 'inside' }).toBuffer();
  await fsPromises.writeFile('../image-sm.jpg', smallBuffer);
};

router.post('/submit', upload.single('file'), (req, res) => {
  processImage(req.file.buffer)
    .then(() => {
      res.json({ success: true });
    })
    .catch((err) => {
      res.status(400).json({ error: err.message })
    });
});
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • I'm importing the promise version of fs like ```var fs = require('fs').promises``` – Glenn Jan 13 '20 at 04:11
  • 1
    Makes sense. Do you know if `image.resize` can handle parallel processing, or can it only work serially? If it can be called in parallel, the speed of the code could be improved with `Promise.all` – CertainPerformance Jan 13 '20 at 04:24
  • so weird, I tried rewriting using async/await and exactly the same result. The double resize corrupts the successful response. I believe it can handle parallel processing, the library author recommends a promise.all, but I am intentionally not doing that because I actually need to do other things in between resizes. – Glenn Jan 13 '20 at 04:30
  • Sounds a bit silly, I know, but are you sure you saved the changes? From the original code, I'd *expect* the Promise chain to hang, but if the `.toBuffer` calls are waited for properly (either `await`ed, or returned from the `.then`), there's no reason for anything to hang, unless there's a bug in sharp or `fs`. Using the `await` version, it should be pretty easy to add `console.log`s to see which line isn't progressing, but that seems odd – CertainPerformance Jan 13 '20 at 04:37
  • I've definitely saved it. :) All I can think is that there is some issue with calling ```.toBuffer()``` twice. If I take the 2nd one out, everything works fine. – Glenn Jan 13 '20 at 04:55
  • "*but you're pass it the result of calling toBuffer on calling image.resize immediately*" - uh, no. The "*using parenthesis*" snippet is not equivalent at all to the OP's code - it calls `toBuffer` on the arrow function object, which would throw an exception. The original code is totally fine, the `toBuffer` call is inside the arrow function concise body expression, it's equivalent to `.then(() => { return image.resize(500, 500, { fit: 'inside' }).toBuffer(); })` – Bergi Jan 13 '20 at 21:51
0

The issue turned out to be my request timeout. ‍♂️

My timeout was set to 1 second, and when I was doing more than one resize operation, it took longer than 1 second. I upped the timeout to 10 seconds and that fixed it.

Glenn
  • 4,195
  • 9
  • 33
  • 41