diff --git a/pkg/oonimkall/taskrunner.go b/pkg/oonimkall/taskrunner.go index 15ea04a..f63eda2 100644 --- a/pkg/oonimkall/taskrunner.go +++ b/pkg/oonimkall/taskrunner.go @@ -291,7 +291,6 @@ func (r *runnerForTask) Run(rootCtx context.Context) { // submit measurement and stop at beginning of next iteration break } - m.AddAnnotations(r.settings.Annotations) if err != nil { r.emitter.Emit(eventTypeFailureMeasurement, eventMeasurementGeneric{ Failure: err.Error(), @@ -304,6 +303,7 @@ func (r *runnerForTask) Run(rootCtx context.Context) { // now the only valid strategy here is to continue. continue } + m.AddAnnotations(r.settings.Annotations) data, err := json.Marshal(m) runtimex.PanicOnError(err, "measurement.MarshalJSON failed") r.emitter.Emit(eventTypeMeasurement, eventMeasurementGeneric{ diff --git a/pkg/oonimkall/taskrunner_test.go b/pkg/oonimkall/taskrunner_test.go index e034366..5e61621 100644 --- a/pkg/oonimkall/taskrunner_test.go +++ b/pkg/oonimkall/taskrunner_test.go @@ -469,6 +469,39 @@ func TestTaskRunnerRun(t *testing.T) { assertReducedEventsLike(t, expect, reduced) }) + t.Run("with measurement failure and annotations", func(t *testing.T) { + // See https://github.com/ooni/probe/issues/2173. We want to be sure that + // we are not crashing when the measurement fails and there are annotations, + // which is what was happening in the above referenced issue. + runner, emitter := newRunnerForTesting() + fake := fakeSuccessfulRun() + fake.MockableInputPolicy = func() engine.InputPolicy { + return engine.InputNone + } + fake.MockableMeasureWithContext = func(ctx context.Context, input string) (measurement *model.Measurement, err error) { + return nil, errors.New("preconditions error") + } + runner.sessionBuilder = fake + runner.settings.Annotations = map[string]string{ + "architecture": "arm64", + } + events := runAndCollect(runner, emitter) + reduced := reduceEventsKeysIgnoreLog(events) + expect := []eventKeyCount{ + {Key: eventTypeStatusQueued, Count: 1}, + {Key: eventTypeStatusStarted, Count: 1}, + {Key: eventTypeStatusProgress, Count: 3}, + {Key: eventTypeStatusGeoIPLookup, Count: 1}, + {Key: eventTypeStatusResolverLookup, Count: 1}, + {Key: eventTypeStatusProgress, Count: 1}, + {Key: eventTypeStatusReportCreate, Count: 1}, + {Key: eventTypeStatusMeasurementStart, Count: 1}, + {Key: eventTypeFailureMeasurement, Count: 1}, + {Key: eventTypeStatusEnd, Count: 1}, + } + assertReducedEventsLike(t, expect, reduced) + }) + t.Run("with success and InputStrictlyRequired", func(t *testing.T) { runner, emitter := newRunnerForTesting() runner.settings.Inputs = []string{"a", "b", "c", "d"}