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

fix: make Runner's concatAll safer #339

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AndersDJohnson
Copy link

@AndersDJohnson AndersDJohnson commented Aug 11, 2019

I have noticed that in some cases the concatAll function in Runner.js results in errors. Looks like the function assumes a fully populated nested array, whereas the code that generates the nested array with which this function is called can actually result in undefined nested items by resolving the promise with nothing here:

resolve();

If it is intentional to throw an error in this case rather than proceeding, I would at least prefer if such errors were handled more gracefully, as I saw no indication as to the root cause in the logs or stack trace when this occurred.

Alternatively, we could change the resolve() to resolve([]), as is done below it. It looks like this was attempted in #334.

Let me know if we should add unit tests to cover this possibility and prove this works - I just wanted to raise a minimal PR to clearly demonstrate the change in functionality to start.

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

Successfully merging this pull request may close these issues.

3 participants