Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

fix _findNProvidersAsync discarding search results #137

Merged
merged 2 commits into from
Jul 29, 2019

Conversation

Qmstream
Copy link
Contributor

When _findNProvidersAsync times out, it ignores the search results, as it's checking the out array before it's actually populated.

Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

@Qmstream nice find! Would you mind adding a test for this? It would be great to avoid any future regressions of this, especially as we're migrating to full async/await code soon.

It looks like linting is failing due to extra white space on line 579.

@Qmstream
Copy link
Contributor Author

@jacobheun, I removed the dangling white space.

But, sorry, I have no idea how I could write the suggested test (I have little experience with this).

@jacobheun
Copy link
Contributor

@Qmstream no problem, I will look at getting a test added for this. I added #138 to track getting the test added so we don't block this, since the test addition isn't critical and it would be nice to have this out sooner.

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM

@vasco-santos
Copy link
Member

Thanks for looking to the PR and creating the issue @jacobheun

@vasco-santos vasco-santos merged commit e656c6b into libp2p:master Jul 29, 2019
@vasco-santos
Copy link
Member

Also @Qmstream thanks for this PR ❤️

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

Successfully merging this pull request may close these issues.

4 participants