Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

fix: remove async from final function which causes double invocation …#2094

Merged
ddelgrosso1 merged 2 commits intogoogleapis:mainfrom
ddelgrosso1:fix-double-cb
Oct 25, 2022
Merged

fix: remove async from final function which causes double invocation …#2094
ddelgrosso1 merged 2 commits intogoogleapis:mainfrom
ddelgrosso1:fix-double-cb

Conversation

@ddelgrosso1
Copy link
Copy Markdown
Contributor

…of callback

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #2082 🦕

@ddelgrosso1 ddelgrosso1 requested review from a team October 24, 2022 18:57
@product-auto-label product-auto-label Bot added size: xs Pull request size is extra small. api: storage Issues related to the googleapis/nodejs-storage API. labels Oct 24, 2022
@ddelgrosso1
Copy link
Copy Markdown
Contributor Author

I believe this is related to nodejs/node#39535 which can cause double invocation of callbacks under the same situation we had in the code here.

@ddelgrosso1 ddelgrosso1 added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 24, 2022
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 24, 2022
@ddelgrosso1 ddelgrosso1 added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 24, 2022
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 24, 2022
@ddelgrosso1 ddelgrosso1 merged commit 1a3df98 into googleapis:main Oct 25, 2022
Comment thread src/file.ts
Comment on lines +1538 to +1541
.then(() => {
cb();
})
.catch(cb);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

couldn't this cause a double invocation if cb throws?

I think I would have used something such as:

.then(
  () => cb(),
  () => cb()
)

Also I'm not entirely sure how the previous code would end up with a double invocation, but I may miss some context.

ddelgrosso1 added a commit to ddelgrosso1/nodejs-storage that referenced this pull request Dec 5, 2022
googleapis#2094)

* fix: remove async from final function which causes double invocation of callback

* add catch clause
ddelgrosso1 added a commit to ddelgrosso1/nodejs-storage that referenced this pull request Dec 5, 2022
googleapis#2094)

* fix: remove async from final function which causes double invocation of callback

* add catch clause
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: storage Issues related to the googleapis/nodejs-storage API. size: xs Pull request size is extra small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ERR_MULTIPLE_CALLBACK on file.download()

3 participants