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: Return node only after go-ipfs daemon is ready #150

Closed
wants to merge 2 commits into from

Conversation

haadcode
Copy link
Member

Testing new ipfsd-ctl with Orbit I found a bug where the node is returned before the gateway address has been received and patched to the returned API. Ie. gatewayAddr was undefined.

This PR changes the callback logic so that we listen for "Daemon is ready" stdout from go-ipfs and only then return. Previously we returned when stdout matched "API listening...".

@haadcode haadcode added the status/in-progress In progress label Mar 22, 2017

if (readyMatch) {
callback(null, this.api)
}
Copy link
Member

Choose a reason for hiding this comment

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

str.match is non blocking, if you found that bug happening, it means that the output was captured before go-ipfs had written everything. I believe that this doesn't solve the issue (🕸🕷), only silences it by not calling the callback with an undefined parameter. Let's add a else to callback with a error signalling that the daemon was not found ready.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this part was getting called multiple times (twice in this case), first with "API listening..." and then with "Gateway (readonly) listening...\nDaemon is ready". Not sure why or if that's the intended behaviour, but adding an else and an error callback will just make it fail always (from user's perspective).

@dignifiedquire I believe you refactored the exec code last time, any ideas here?

Copy link
Member

Choose a reason for hiding this comment

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

You can use http://npmjs.org/once

@daviddias daviddias added status/ready Ready to be worked and removed status/in-progress In progress labels Apr 5, 2017
@daviddias daviddias mentioned this pull request Sep 6, 2017
@daviddias
Copy link
Member

daviddias commented Sep 6, 2017

@haadcode just completed the task of slaying the very big dragon that was dormant in ipfsd-ctl, the same you tried to get to in this PR. See how I solved it here: #165. This was a fun one 🐲

@daviddias daviddias closed this Sep 6, 2017
@daviddias daviddias deleted the fix/gatewayAddr branch September 6, 2017 10:30
@haadcode
Copy link
Member Author

haadcode commented Sep 6, 2017

Awesome! Thank you for slaying the 🐉 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/ready Ready to be worked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants