feat: test hook error handling

docs/service_spec.md

@@ -147,6 +147,11 @@ A test hook must:
       * `stage` (string, optional): If executing a stage, for example `beforeEvaluation`, this should be the stage.
   - Return data from the stages as specified via the `data` configuration. For instance the return value from the `beforeEvaluation` hook should be `data['beforeEvaluation']` merged with the input data for the stage.
 
+### Capability `evaluation-hooks:errors`
+
+This capability is only used if `evaluation-hooks` is enabled. It means that the SDK is capable of catching exceptions or errors generated within stage handlers of a hook,
+and is able to continue on without issue after handling that exception/error.
+
 ### Stop test service: `DELETE /`
 
 The test harness sends this request at the end of a test run if you have specified `--stop-service-at-end` on the [command line](./running.md). The test service should simply quit. This is a convenience so CI scripts can simply start the test service in the background and assume it will be stopped for them.
@@ -194,6 +199,9 @@ A `POST` request indicates that the test harness wants to start an instance of t
       * `data` (object, optional): Contains data which should return from different execution stages.
         * `beforeEvaluation` (object, optional): A map of `string` to `ldvalue` items. This should be returned from the `beforeEvaluation` stage of the test hook.
         * `afterEvaluation` (object, optional): A map of `string` to `ldvalue` items. This should be returned from the `afterEvaluation` stage of the test hook.
+      * `errors` (object, optional): Specifies that an error should be returned/exception thrown from a stage.
+        * `beforeEvaluation` (string, optional): The error/exception message that should be generated in the `beforeEvaluation` stage of the test hook.
+        * `afterEvaluation` (string, optional): The error/exception message that should be generated in the `afterEvaluation` stage of the test hook.
 
 The response to a valid request is any HTTP `2xx` status, with a `Location` header whose value is the URL of the test service resource representing this SDK client instance (that is, the one that would be used for "Close client" or "Send command" as described below).
 

sdktests/server_side_hooks.go

@@ -22,6 +22,8 @@ func doServerSideHooksTests(t *ldtest.T) {
 	t.Run("executes afterEvaluation stage", executesAfterEvaluationStage)
 	t.Run("data propagates from before to after", beforeEvaluationDataPropagatesToAfter)
 	t.Run("data propagates from before to after for migrations", beforeEvaluationDataPropagatesToAfterMigration)
+	t.Run("an error in one stage does not prevent others from executing", errorsDoNotAffectSubsequentStages)
+
 }
 
 func executesBeforeEvaluationStage(t *ldtest.T) {
@@ -269,8 +271,56 @@ func beforeEvaluationDataPropagatesToAfterMigration(t *ldtest.T) {
 	})
 }
 
+// This test is meant to check Requirement HOOKS:1.3.7.
+func errorsDoNotAffectSubsequentStages(t *ldtest.T) {
+	t.RequireCapability(servicedef.CapabilityEvaluationHookErrors)
+
+	hookName := "fallibleHook"
+
+	// We're configuring the beforeEvaluation stage with some data, but we don't expect to see it propagated into afterEvaluation
+	// since we're also configuring beforeEvaluation to throw an exception (or return an error, whatever is appropriate for the language.)
+	hookData := map[servicedef.HookStage]servicedef.SDKConfigEvaluationHookData{
+		servicedef.BeforeEvaluation: map[string]ldvalue.Value{"this_value": ldvalue.String("should_not_be_received")},
+	}
+
+	client, hooks := createClientForHooksWithErrors(t, []string{hookName}, hookData, map[servicedef.HookStage]o.Maybe[string]{
+		servicedef.BeforeEvaluation: o.Some("something is rotten in the state of Denmark!"),
+	})
+
+	defer hooks.Close()
+
+	flagKey := "bool-flag"
+	client.EvaluateFlag(t, servicedef.EvaluateFlagParams{
+		FlagKey:      flagKey,
+		Context:      o.Some(ldcontext.New("user-key")),
+		ValueType:    servicedef.ValueTypeBool,
+		DefaultValue: ldvalue.Bool(false),
+	})
+
+	hooks.ExpectCall(t, hookName, func(payload servicedef.HookExecutionPayload) bool {
+		if payload.Stage.Value() == servicedef.BeforeEvaluation {
+			t.Errorf("SDK implementation error: beforeEvaluation should not have caused a POST to the test harness; ensure exception is thrown/error returned in this stage")
+			return false
+		}
+		if payload.Stage.Value() == servicedef.AfterEvaluation {
+			// Requirement HOOKS:1.3.7.1 says that:
+			// "The client should use the data from the previous successful stage, or empty data if there is no previous stage."
+			// Since there are no other preceding stages besides beforeEvaluation, then the data should be empty.
+			hookData := payload.EvaluationSeriesData.Value()
+			assert.Len(t, hookData, 0)
+			return true
+		}
+		return false
+	})
+}
+
 func createClientForHooks(t *ldtest.T, instances []string,
 	hookData map[servicedef.HookStage]servicedef.SDKConfigEvaluationHookData) (*SDKClient, *Hooks) {
+	return createClientForHooksWithErrors(t, instances, hookData, nil)
+}
+
+func createClientForHooksWithErrors(t *ldtest.T, instances []string,
+	hookData map[servicedef.HookStage]servicedef.SDKConfigEvaluationHookData, hookErrors map[servicedef.HookStage]o.Maybe[string]) (*SDKClient, *Hooks) {
 	boolFlag := ldbuilders.NewFlagBuilder("bool-flag").
 		Variations(ldvalue.Bool(false), ldvalue.Bool(true)).
 		FallthroughVariation(1).On(true).Build()
@@ -296,7 +346,7 @@ func createClientForHooks(t *ldtest.T, instances []string,
 	dataBuilder := mockld.NewServerSDKDataBuilder()
 	dataBuilder.Flag(boolFlag, numberFlag, stringFlag, jsonFlag, migrationFlag)
 
-	hooks := NewHooks(requireContext(t).harness, t.DebugLogger(), instances, hookData)
+	hooks := NewHooks(requireContext(t).harness, t.DebugLogger(), instances, hookData, hookErrors)
 
 	dataSource := NewSDKDataSource(t, dataBuilder.Build())
 	events := NewSDKEventSink(t)

sdktests/testapi_hooks.go

@@ -18,6 +18,7 @@ type HookInstance struct {
 	name        string
 	hookService *mockld.HookCallbackService
 	data        map[servicedef.HookStage]servicedef.SDKConfigEvaluationHookData
+	errors      map[servicedef.HookStage]o.Maybe[string]
 }
 
 type Hooks struct {
@@ -29,6 +30,7 @@ func NewHooks(
 	logger framework.Logger,
 	instances []string,
 	data map[servicedef.HookStage]servicedef.SDKConfigEvaluationHookData,
+	errors map[servicedef.HookStage]o.Maybe[string],
 ) *Hooks {
 	hooks := &Hooks{
 		instances: make(map[string]HookInstance),
@@ -38,6 +40,7 @@ func NewHooks(
 			name:        instance,
 			hookService: mockld.NewHookCallbackService(testHarness, logger),
 			data:        data,
+			errors:      errors,
 		}
 	}
 
@@ -51,6 +54,7 @@ func (h *Hooks) Configure(config *servicedef.SDKConfigParams) error {
 			Name:        instance.name,
 			CallbackURI: instance.hookService.GetURL(),
 			Data:        instance.data,
+			Errors:      instance.errors,
 		})
 	}
 	config.Hooks = o.Some(hookConfig)

servicedef/sdk_config.go

@@ -84,6 +84,7 @@ type SDKConfigHookInstance struct {
 	Name        string                                    `json:"name"`
 	CallbackURI string                                    `json:"callbackUri"`
 	Data        map[HookStage]SDKConfigEvaluationHookData `json:"data,omitempty"`
+	Errors      map[HookStage]o.Maybe[string]             `json:"errors,omitempty"`
 }
 
 type SDKConfigHooksParams struct {

servicedef/service_params.go

@@ -16,23 +16,24 @@ const (
 	CapabilityAllFlagsClientSideOnly             = "all-flags-client-side-only"
 	CapabilityAllFlagsDetailsOnlyForTrackedFlags = "all-flags-details-only-for-tracked-flags"
 
-	CapabilityBigSegments        = "big-segments"
-	CapabilityContextType        = "context-type"
-	CapabilityContextComparison  = "context-comparison"
-	CapabilitySecureModeHash     = "secure-mode-hash"
-	CapabilityServerSidePolling  = "server-side-polling"
-	CapabilityServiceEndpoints   = "service-endpoints"
-	CapabilityTags               = "tags"
-	CapabilityUserType           = "user-type"
-	CapabilityFiltering          = "filtering"
-	CapabilityAutoEnvAttributes  = "auto-env-attributes"
-	CapabilityMigrations         = "migrations"
-	CapabilityEventSampling      = "event-sampling"
-	CapabilityETagCaching        = "etag-caching"
-	CapabilityInlineContext      = "inline-context"
-	CapabilityAnonymousRedaction = "anonymous-redaction"
-	CapabilityPollingGzip        = "polling-gzip"
-	CapabilityEvaluationHooks    = "evaluation-hooks"
+	CapabilityBigSegments          = "big-segments"
+	CapabilityContextType          = "context-type"
+	CapabilityContextComparison    = "context-comparison"
+	CapabilitySecureModeHash       = "secure-mode-hash"
+	CapabilityServerSidePolling    = "server-side-polling"
+	CapabilityServiceEndpoints     = "service-endpoints"
+	CapabilityTags                 = "tags"
+	CapabilityUserType             = "user-type"
+	CapabilityFiltering            = "filtering"
+	CapabilityAutoEnvAttributes    = "auto-env-attributes"
+	CapabilityMigrations           = "migrations"
+	CapabilityEventSampling        = "event-sampling"
+	CapabilityETagCaching          = "etag-caching"
+	CapabilityInlineContext        = "inline-context"
+	CapabilityAnonymousRedaction   = "anonymous-redaction"
+	CapabilityPollingGzip          = "polling-gzip"
+	CapabilityEvaluationHooks      = "evaluation-hooks"
+	CapabilityEvaluationHookErrors = "evaluation-hooks:errors"
 )
 
 type StatusRep struct {