Skip to content

Commit

Permalink
fix(cmd): nil pointer dereference in PrintProgressBarsOnEvents
Browse files Browse the repository at this point in the history
We now call on `o.Instance()` to get access to the underlying dataset methods (at the lib level). However, this leads to a `nil pointer dereference` error, when we use the CLI to interact with the qri node over http rpc.

`o.Instance()` first calls `o.Init()`, which creates and sets up an instance if we don't currently have one using `lib.NewInstance`. When using the CLI to interact with a qri node via http rpc, however, `lib.NewInstance` returns early after setting up an `HTTPClient` in `inst.http`, so we route any CLI calls to the running qri node. `o.Init()` continues, but expects an instance with a `bus`, so it can pass the bus to `PrintProgressBarOnEvents`, which gives feedback to the user via progress bars.

This bus is nil, so when `PrintProgressBarOnEvents` tries to subscribe to events on the bus, we get the error in question. Because we will probably want to give users progress via http rpc at some point, rather than disabling this behavior via rpc, the current fix just guards against a nil bus before attempting to subscribe to any events.
  • Loading branch information
ramfox committed Mar 23, 2021
1 parent b1fab21 commit b0a2ec0
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
4 changes: 4 additions & 0 deletions cmd/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ func PrintProgressBarsOnEvents(w io.Writer, bus event.Bus) {
p := mpb.New(mpb.WithWidth(80), mpb.WithOutput(w))
progress := map[string]*mpb.Bar{}

if bus == nil {
log.Errorf("event bus is nil")
return
}
// wire up a subscription to print download progress to streams
bus.SubscribeTypes(func(_ context.Context, e event.Event) error {
lock.Lock()
Expand Down
4 changes: 4 additions & 0 deletions cmd/qri.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ func (o *QriOptions) Init() (err error) {
// being false also disables progress bars, which may be what we want (ahem: TTY
// detection), but even if so, isn't the right use of this variable name
if shouldColorOutput {
// TODO(ramfox): we guard for a nil bus in `PrintProgressBarsOnEvents`
// but noting here that no requests that go through http rpc will have
// a working bus, so we won't get any progress bars when working over
// http rpc until this is adjusted (once we get the events "rpc-ified")
PrintProgressBarsOnEvents(o.IOStreams.ErrOut, o.inst.Bus())
}

Expand Down

0 comments on commit b0a2ec0

Please sign in to comment.