Faster Error Messages

function/make.go

@@ -21,6 +21,8 @@ import (
 	"time"
 
 	"github.com/square/metrics/api"
+
+	netcontext "golang.org/x/net/context"
 )
 
 // MakeFunction is a convenient way to use type-safe functions to
@@ -94,24 +96,29 @@ func MakeFunction(name string, function interface{}) MetricFunction {
 				return arg
 			}
 
+			subCtx, cancel := netcontext.WithCancel(context.Ctx())
+			defer cancel() // when done, make sure to terminate the subcontext
+			subcontext := context
+			subcontext.private.Ctx = subCtx
+
 			// evalTo takes an expression and a reflect.Type and evaluates to the appropriate type.
 			// If an Expression is requested, it just returns it.
 			evalTo := func(expression Expression, resultType reflect.Type) (interface{}, error) {
 				switch resultType {
 				case expressionType:
 					return expression, nil
 				case stringType:
-					return EvaluateToString(expression, context)
+					return EvaluateToString(expression, subcontext)
 				case scalarType:
-					return EvaluateToScalar(expression, context)
+					return EvaluateToScalar(expression, subcontext)
 				case scalarSetType:
-					return EvaluateToScalarSet(expression, context)
+					return EvaluateToScalarSet(expression, subcontext)
 				case durationType:
-					return EvaluateToDuration(expression, context)
+					return EvaluateToDuration(expression, subcontext)
 				case timeseriesType:
-					return EvaluateToSeriesList(expression, context)
+					return EvaluateToSeriesList(expression, subcontext)
 				case valueType:
-					return expression.Evaluate(context)
+					return expression.Evaluate(subcontext)
 				}
 				panic(fmt.Sprintf("Unreachable :: Attempting to evaluate to unknown type %+v", resultType))
 			}
@@ -142,9 +149,9 @@ func MakeFunction(name string, function interface{}) MetricFunction {
 				argType := funcType.In(i)
 				switch argType {
 				case contextType:
-					argumentFuncs[i] = provideValue(context)
+					argumentFuncs[i] = provideValue(subcontext)
 				case timerangeType:
-					argumentFuncs[i] = provideValue(context.Timerange())
+					argumentFuncs[i] = provideValue(subcontext.Timerange())
 				case groupsType:
 					argumentFuncs[i] = provideValue(groups)
 				case stringType, scalarType, scalarSetType, durationType, timeseriesType, valueType, expressionType:
@@ -179,19 +186,31 @@ func MakeFunction(name string, function interface{}) MetricFunction {
 				i := i
 				waiter.Add(1)
 				go func() {
-					defer waiter.Done()
 					arg, err := argumentFuncs[i]()
 					if err != nil {
+						// Do NOT .Done() the waiter before sending an error: doing so will
+						// introduce a race between the waiter finishing and the error being
+						// received.
 						errors <- err
 						return
 					}
 					argValues[i] = reflect.ValueOf(arg)
+					waiter.Done()
 				}()
 			}
-			waiter.Wait() // Wait for all the arguments to be evaluated.
-
-			if len(errors) != 0 {
-				return nil, <-errors
+			waitChan := make(chan struct{})
+			go func() {
+				waiter.Wait() // Wait for all the arguments to be evaluated.
+				close(waitChan)
+			}()
+
+			select {
+			case <-waitChan:
+				// Everything completed okay and without error.
+			case err := <-errors:
+				// An error occurred, so we return it immediately.
+				// The deferred cancel will run as the function returns.
+				return nil, err
 			}
 
 			output := funcValue.Call(argValues)

query/tests/command_select_test.go

@@ -520,3 +520,29 @@ func TestTag(t *testing.T) {
 		}
 	}
 }
+
+func TestFastError(t *testing.T) {
+	testTimerange, err := api.NewTimerange(0, 1000, 10)
+	if err != nil {
+		t.Fatalf("unexpected error creating test timerange: %s", err.Error())
+	}
+	comboAPI := mocks.NewComboAPI(testTimerange)
+	cmd, err := parser.Parse("select blah + series_timeout from -30m to now")
+	if err != nil {
+		t.Fatalf("unexpected error parsing test query: %s", err.Error())
+	}
+	now := time.Now()
+	garbage, err := cmd.Execute(command.ExecutionContext{
+		TimeseriesStorageAPI: comboAPI,
+		MetricMetadataAPI:    comboAPI,
+		FetchLimit:           100,
+		Ctx:                  context.Background(),
+	})
+	if err == nil {
+		t.Errorf("expected error from test query but got out %+v", garbage)
+	}
+	if time.Since(now) > time.Millisecond*10 {
+		t.Errorf("expected instantaneous failure, but took %+v time", time.Since(now))
+	}
+
+}