Skip to content

Commit

Permalink
fix(cli/rm/rm.go): using panic is not user friendly (#1608)
Browse files Browse the repository at this point in the history
I manually tested the cli and concluded that it's too easy to reach the
panic caused by runtimex.Try0 over there and it would expose panics to
users too frequently.

Let's just convert those into normal error checks. Here is what we would
previously get:

```
% ./ooniprobe rm
? Are you sure you wish to delete the result #0  [Use arrows to move, type to filter]
  true
> false
panic: Try0: interrupt

[...]
```

So, here I just pressed ^D and got a panic. Ouch!

After the diff, we have this:

```
% ./ooniprobe rm
? Are you sure you wish to delete the result #0 false
   ⨯ failure in main command   error=canceled by user
```

and:

```
% ./ooniprobe rm --all
? Are you sure you wish to delete ALL results false
   ⨯ failure in main command   error=canceled by user
```

It seems much better, isn't it?

Part of the QA for ooni/probe#2722
  • Loading branch information
bassosimone authored May 17, 2024
1 parent 58a286a commit 5493975
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions cmd/ooniprobe/internal/cli/rm/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/apex/log"
"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/cli/root"
"github.com/ooni/probe-cli/v3/internal/database"
"github.com/ooni/probe-cli/v3/internal/runtimex"
"github.com/upper/db/v4"
)

Expand All @@ -21,7 +20,7 @@ func deleteAll(d *database.Database, skipInteractive bool) error {
Options: []string{"true", "false"},
Default: "false",
}
runtimex.Try0(survey.AskOne(confirm, &answer, nil))
_ = survey.AskOne(confirm, &answer, nil) // no error checking: we rely on the default value
if answer == "false" {
return errors.New("canceled by user")
}
Expand Down Expand Up @@ -81,7 +80,7 @@ func init() {
Options: []string{"true", "false"},
Default: "false",
}
runtimex.Try0(survey.AskOne(confirm, &answer, nil))
_ = survey.AskOne(confirm, &answer, nil) // no error checking: we rely on the default value
if answer == "false" {
return errors.New("canceled by user")
}
Expand Down

0 comments on commit 5493975

Please sign in to comment.