Skip to content

Conversation

@grayside
Copy link
Collaborator

@grayside grayside commented Aug 9, 2019

When I run the index.js code from the Functions imagemagick sample in Cloud Run on a Node 10 container, I get the following error:

TypeError [ERR_INVALID_CALLBACK]: Callback must be a function 
     at makeCallback (fs.js:136:11) 
     at Object.unlink (fs.js:943:14) 
     at blurImage (/usr/src/app/image.js:106:23) 
     at process._tickCallback (internal/process/next_tick.js:68:7) 

Looking at the current promisify implementation around fs.unlink, it differs from what I've read in trying to return the result of the promisify. Taking a note from https://dev.to/mrm8488/from-callbacks-to-fspromises-to-handle-the-file-system-in-nodejs-56p2, with this change I do not see any errors.

@grayside grayside requested a review from ace-n August 9, 2019 21:07
@grayside grayside requested a review from grant as a code owner August 9, 2019 21:07
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 9, 2019
// Delete the temporary file.
return promisify(fs.unlink(tempLocalPath));
const unlink = promisify(fs.unlink);
return unlink(tempLocalPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume return promisify(fs.unlink)(tempLocalPath) is either too complex or doesn't work here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't test that, but it seems likely to work.
My reasoning is that this is more explicit, happy to change if you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like explicit samples 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants