Skip to content

Commit

Permalink
fix(miniooni): handle panics with --repeat-every (#914)
Browse files Browse the repository at this point in the history
Most of miniooni panics on errors. We should not panic on error with
--repeat-every, rather we should try the next measurement.

See ooni/probe#2250
  • Loading branch information
bassosimone authored Aug 31, 2022
1 parent 0bc6aae commit a8a29cc
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
6 changes: 5 additions & 1 deletion internal/cmd/miniooni/consent.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ func acquireUserConsent(miniooniDir string, currentOptions *Options) {
consentFile := path.Join(miniooniDir, "informed")
err := maybeWriteConsentFile(currentOptions.Yes, consentFile)
runtimex.PanicOnError(err, "cannot write informed consent file")
runtimex.PanicIfFalse(regularFileExists(consentFile), riskOfRunningOONI)
runtimex.PanicIfFalse(
regularFileExists(consentFile),
riskOfRunningOONI,
)
}

// maybeWriteConsentFile writes the consent file iff the yes argument is true
Expand All @@ -30,6 +33,7 @@ func maybeWriteConsentFile(yes bool, filepath string) (err error) {

// riskOfRunningOONI is miniooni's informed consent text.
const riskOfRunningOONI = `
Do you consent to OONI Probe data collection?
OONI Probe collects evidence of internet censorship and measures
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"os"
"path"
"runtime/debug"
"strings"
"time"

Expand Down Expand Up @@ -287,6 +288,17 @@ func MainWithConfiguration(experimentName string, currentOptions *Options) {
// mainSingleIteration runs a single iteration. There may be multiple iterations
// when the user specifies the --repeat-every command line flag.
func mainSingleIteration(logger model.Logger, experimentName string, currentOptions *Options) {

// We allow the inner code to fail but we stop propagating the panic here
// such that --repeat-every works as intended anyway
if currentOptions.RepeatEvery > 0 {
defer func() {
if r := recover(); r != nil {
log.Warnf("recovered from panic: %+v\n%s\n", r, debug.Stack())
}
}()
}

extraOptions := mustMakeMapStringAny(currentOptions.ExtraOptions)
annotations := mustMakeMapStringString(currentOptions.Annotations)

Expand Down

0 comments on commit a8a29cc

Please sign in to comment.