Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: swallowed errors #1860

Merged
merged 4 commits into from
Feb 6, 2019
Merged

fix: swallowed errors #1860

merged 4 commits into from
Feb 6, 2019

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Jan 31, 2019

The switch to using yargs-promise for ipfs init and ipfs daemon commands caused an unhandled promise rejection and in some cases would cause an error to not be printed to the console.

This PR greatly simplifies the code in src/cli/bin.js, to always use yargs-promise. Command handlers are now passed an async getIpfs function instead of an ipfs instance. It means that we don't have to differentiate between commands that use an IPFS instance in src/cli/bin.js, giving the handler the power to call getIpfs or not to obtain an IPFS instance as and when needed. This removes a whole bunch of complexity from src/cli/bin.js at the cost of adding a single line to every command handler that needs to use an IPFS instance.

This enables operations like echo "hello" | jsipfs add -q | jsipfs cid base32 to work without jsipfs cid base32 failing because it's trying to acquire a repo lock when it doesn't use IPFS at all.

fixes #1835
refs #1858
refs libp2p/js-libp2p#311

@ghost ghost assigned alanshaw Jan 31, 2019
@ghost ghost added the status/in-progress In progress label Jan 31, 2019
@alanshaw alanshaw changed the title fix: swallowed errors [WIP] fix: swallowed errors Jan 31, 2019
@alanshaw alanshaw changed the title [WIP] fix: swallowed errors fix: swallowed errors Jan 31, 2019
The switch to using `yargs-promise` for `ipfs init` and `ipfs daemon` commands caused an unhandled promise rejection and in some cases would cause an error to not be printed to the console.

This PR greatly simplifies the code in `src/cli/bin.js`, to always use `yargs-promise`. Command handlers are now passed an async `getIpfs` function instead of an `ipfs` instance. It means that we don't have to differentiate between commands that use an IPFS instance in `src/cli/bin.js`, giving the handler the power to call `getIpfs` or not to obtain an IPFS instance as and when needed. This removes a whole bunch of complexity from `src/cli/bin.js` at the cost of adding a single line to every command handler that needs to use an IPFS instance.

This enables operations like `echo "hello" | jsipfs add -q | jsipfs cid base32` to work without `jsipfs cid base32` failing because it's trying to acquire a repo lock when it doesn't use IPFS at all.

fixes #1835
refs #1858
refs libp2p/js-libp2p#311

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Pipe to self: "Error: Lock file is already being hold"
2 participants