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

Don't stop all collectors if one fails #13

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

liamphmurphy
Copy link
Contributor

No description provided.

@liamphmurphy liamphmurphy changed the title don't stop all collectors if one fails Don't stop all collectors if one fails Sep 5, 2022
Copy link

@mindblight mindblight left a comment

Choose a reason for hiding this comment

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

I know a lot about JS and promises, but I'm new to CodeForPX so I don't feel quite ready to push approve :)

src/index.js Outdated
Comment on lines 83 to 89
// Do not reject all promises when one of them is rejected, but do output why one was rejected.
Promise.allSettled(promises)
.then((results) => results.forEach((result) => {
if(result.status == "rejected") {
console.log("received a rejected promise with reason: " + result.reason)
}
}));

Choose a reason for hiding this comment

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

💬 This works great. You can also use Javascript's async/await for the same effect:

const results = await promises;
for (const result of results) {
  if (result.status === "rejected") {
    ...
  }
}

src/writeHelper.js Outdated Show resolved Hide resolved
Co-authored-by: Arion Sprague <[email protected]>
}
console.log("wrote", fileName)
});
return writeFile(fileName, JSON.stringify(data))

Choose a reason for hiding this comment

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

❓ Does this work without importing writeFile?

import { writeFile } from 'fs/promises';`

Choose a reason for hiding this comment

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

Ah, sorry. Just realized this project is using CommonJS.

const { writeFile } = require('fs/promises);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep the second way seems to work as expected, thanks for the suggestion 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants