From 02df17df083cc63af22189d5de59e6b8ff986f13 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Thu, 28 Mar 2024 11:14:54 -0700
Subject: [PATCH 01/11] feat: test hook error handling

---
 docs/service_spec.md          |  8 ++++++
 sdktests/server_side_hooks.go | 52 ++++++++++++++++++++++++++++++++++-
 sdktests/testapi_hooks.go     |  4 +++
 servicedef/sdk_config.go      |  1 +
 servicedef/service_params.go  | 35 +++++++++++------------
 5 files changed, 82 insertions(+), 18 deletions(-)

diff --git a/docs/service_spec.md b/docs/service_spec.md
index 4cdafae..b1ba27b 100644
--- a/docs/service_spec.md
+++ b/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).
 
diff --git a/sdktests/server_side_hooks.go b/sdktests/server_side_hooks.go
index 68a6ccb..5b38ced 100644
--- a/sdktests/server_side_hooks.go
+++ b/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)
diff --git a/sdktests/testapi_hooks.go b/sdktests/testapi_hooks.go
index 95ccdfa..24d34fc 100644
--- a/sdktests/testapi_hooks.go
+++ b/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)
diff --git a/servicedef/sdk_config.go b/servicedef/sdk_config.go
index ec0a6f3..d15dfbd 100644
--- a/servicedef/sdk_config.go
+++ b/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 {
diff --git a/servicedef/service_params.go b/servicedef/service_params.go
index c2b3f04..df8c14c 100644
--- a/servicedef/service_params.go
+++ b/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 {

From 038084feb25e5640a27d2572f8b25a9c76feb4b7 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Thu, 28 Mar 2024 11:34:43 -0700
Subject: [PATCH 02/11] line length

---
 sdktests/server_side_hooks.go | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/sdktests/server_side_hooks.go b/sdktests/server_side_hooks.go
index 5b38ced..6d6d70e 100644
--- a/sdktests/server_side_hooks.go
+++ b/sdktests/server_side_hooks.go
@@ -277,8 +277,9 @@ func errorsDoNotAffectSubsequentStages(t *ldtest.T) {
 
 	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.)
+	// 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")},
 	}
@@ -299,13 +300,17 @@ func errorsDoNotAffectSubsequentStages(t *ldtest.T) {
 
 	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")
+			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.
+			// "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

From a1e932793602bad908edc7fa39de02d82c6c7a81 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 12:58:19 -0700
Subject: [PATCH 03/11] updating error test to run against N hooks

---
 docs/service_spec.md          |  8 ------
 sdktests/server_side_hooks.go | 46 ++++++++++++++++-------------------
 sdktests/testapi_hooks.go     | 22 +++++++++++++++++
 servicedef/service_params.go  | 35 +++++++++++++-------------
 4 files changed, 60 insertions(+), 51 deletions(-)

diff --git a/docs/service_spec.md b/docs/service_spec.md
index b1ba27b..4cdafae 100644
--- a/docs/service_spec.md
+++ b/docs/service_spec.md
@@ -147,11 +147,6 @@ 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.
@@ -199,9 +194,6 @@ 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).
 
diff --git a/sdktests/server_side_hooks.go b/sdktests/server_side_hooks.go
index 6d6d70e..e465626 100644
--- a/sdktests/server_side_hooks.go
+++ b/sdktests/server_side_hooks.go
@@ -4,6 +4,7 @@ import (
 	"github.com/launchdarkly/go-sdk-common/v3/ldcontext"
 	"github.com/launchdarkly/go-sdk-common/v3/ldmigration"
 	"github.com/launchdarkly/go-sdk-common/v3/ldvalue"
+	"strconv"
 
 	"github.com/launchdarkly/go-server-sdk-evaluation/v3/ldbuilders"
 
@@ -22,8 +23,7 @@ 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)
-
+	t.Run("an error in before stage does not affect after stage", errorInBeforeStageDoesNotAffectAfterStage)
 }
 
 func executesBeforeEvaluationStage(t *ldtest.T) {
@@ -272,10 +272,9 @@ 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)
+func errorInBeforeStageDoesNotAffectAfterStage(t *ldtest.T) {
 
-	hookName := "fallibleHook"
+	const numHooks = 100 // why not?
 
 	// 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
@@ -284,7 +283,12 @@ func errorsDoNotAffectSubsequentStages(t *ldtest.T) {
 		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]{
+	var names []string
+	for i := 0; i < numHooks; i++ {
+		names = append(names, "fallibleHook-"+strconv.Itoa(i))
+	}
+
+	client, hooks := createClientForHooksWithErrors(t, names, hookData, map[servicedef.HookStage]o.Maybe[string]{
 		servicedef.BeforeEvaluation: o.Some("something is rotten in the state of Denmark!"),
 	})
 
@@ -298,25 +302,17 @@ func errorsDoNotAffectSubsequentStages(t *ldtest.T) {
 		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
-	})
+	const numAfterCalls = numHooks
+	calls := hooks.ExpectSingleCallForEachHook(t, names, numAfterCalls)
+
+	for _, call := range calls {
+		assert.Equal(t, servicedef.AfterEvaluation, call.Stage.Value(), "HOOKS:1.3.7: beforeEvaluation "+
+			"should not have caused a POST to the test harness; ensure exception is thrown/error "+
+			"returned in this stage")
+
+		assert.Equal(t, 0, len(call.EvaluationSeriesData.Value()), "HOOKS:1.3.7.1: Since "+
+			"beforeEvaluation should have failed, the data passed to afterEvaluation should be an empty string")
+	}
 }
 
 func createClientForHooks(t *ldtest.T, instances []string,
diff --git a/sdktests/testapi_hooks.go b/sdktests/testapi_hooks.go
index 24d34fc..21999e1 100644
--- a/sdktests/testapi_hooks.go
+++ b/sdktests/testapi_hooks.go
@@ -1,6 +1,7 @@
 package sdktests
 
 import (
+	"github.com/stretchr/testify/assert"
 	"time"
 
 	"github.com/launchdarkly/sdk-test-harness/v2/framework"
@@ -82,3 +83,24 @@ func (h *Hooks) ExpectCall(t *ldtest.T, hookName string,
 		}
 	}
 }
+
+func (h *Hooks) ExpectSingleCallForEachHook(t *ldtest.T, hookNames []string, count int) []servicedef.HookExecutionPayload {
+	out := make(chan o.Maybe[servicedef.HookExecutionPayload])
+
+	for _, hookName := range hookNames {
+		go func(name string) {
+			out <- helpers.TryReceive(h.instances[name].hookService.CallChannel, hookReceiveTimeout)
+		}(hookName)
+	}
+
+	payloads := make([]servicedef.HookExecutionPayload, 0)
+	for i := 0; i < count; i++ {
+		if val := <-out; val.IsDefined() {
+			payloads = append(payloads, val.Value())
+		}
+	}
+
+	assert.Len(t, payloads, count, "Expected %d hook calls, got %d", count, len(payloads))
+
+	return payloads
+}
diff --git a/servicedef/service_params.go b/servicedef/service_params.go
index df8c14c..c2b3f04 100644
--- a/servicedef/service_params.go
+++ b/servicedef/service_params.go
@@ -16,24 +16,23 @@ 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"
-	CapabilityEvaluationHookErrors = "evaluation-hooks:errors"
+	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"
 )
 
 type StatusRep struct {

From 3357e7b95ff89bed814f64d263298859a98599b7 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 13:00:13 -0700
Subject: [PATCH 04/11] update docs

---
 docs/service_spec.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/service_spec.md b/docs/service_spec.md
index 4cdafae..89b9580 100644
--- a/docs/service_spec.md
+++ b/docs/service_spec.md
@@ -194,6 +194,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. The error message itself is not tested by the framework at this time, as it is not a specified behavior.
+        * `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).
 

From 8e0f4753bf5c3c99fdeb951da2ecc954258fdf29 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 13:02:16 -0700
Subject: [PATCH 05/11] more docs

---
 sdktests/server_side_hooks.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sdktests/server_side_hooks.go b/sdktests/server_side_hooks.go
index e465626..d7634b1 100644
--- a/sdktests/server_side_hooks.go
+++ b/sdktests/server_side_hooks.go
@@ -271,7 +271,9 @@ func beforeEvaluationDataPropagatesToAfterMigration(t *ldtest.T) {
 	})
 }
 
-// This test is meant to check Requirement HOOKS:1.3.7.
+// This test is meant to check Requirement HOOKS:1.3.7:
+// The client MUST handle exceptions which are thrown (or errors returned, if idiomatic for the language)
+// during the execution of a stage or handler allowing operations to complete unaffected.
 func errorInBeforeStageDoesNotAffectAfterStage(t *ldtest.T) {
 
 	const numHooks = 100 // why not?
@@ -302,6 +304,8 @@ func errorInBeforeStageDoesNotAffectAfterStage(t *ldtest.T) {
 		DefaultValue: ldvalue.Bool(false),
 	})
 
+	// Since we shouldn't receive any beforeEvaluation calls, the number of calls to expect is simply
+	// the number of hooks configured.
 	const numAfterCalls = numHooks
 	calls := hooks.ExpectSingleCallForEachHook(t, names, numAfterCalls)
 

From 2577036545665c5c1b5cce48c99caee33cd38d67 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 13:08:13 -0700
Subject: [PATCH 06/11] refactor expect call

---
 sdktests/server_side_hooks.go |  6 +-----
 sdktests/testapi_hooks.go     | 12 ++++++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/sdktests/server_side_hooks.go b/sdktests/server_side_hooks.go
index d7634b1..425c1ad 100644
--- a/sdktests/server_side_hooks.go
+++ b/sdktests/server_side_hooks.go
@@ -275,7 +275,6 @@ func beforeEvaluationDataPropagatesToAfterMigration(t *ldtest.T) {
 // The client MUST handle exceptions which are thrown (or errors returned, if idiomatic for the language)
 // during the execution of a stage or handler allowing operations to complete unaffected.
 func errorInBeforeStageDoesNotAffectAfterStage(t *ldtest.T) {
-
 	const numHooks = 100 // why not?
 
 	// We're configuring the beforeEvaluation stage with some data, but we don't expect
@@ -304,10 +303,7 @@ func errorInBeforeStageDoesNotAffectAfterStage(t *ldtest.T) {
 		DefaultValue: ldvalue.Bool(false),
 	})
 
-	// Since we shouldn't receive any beforeEvaluation calls, the number of calls to expect is simply
-	// the number of hooks configured.
-	const numAfterCalls = numHooks
-	calls := hooks.ExpectSingleCallForEachHook(t, names, numAfterCalls)
+	calls := hooks.ExpectAtLeastOneCallForEachHook(t, names)
 
 	for _, call := range calls {
 		assert.Equal(t, servicedef.AfterEvaluation, call.Stage.Value(), "HOOKS:1.3.7: beforeEvaluation "+
diff --git a/sdktests/testapi_hooks.go b/sdktests/testapi_hooks.go
index 21999e1..82ae6ff 100644
--- a/sdktests/testapi_hooks.go
+++ b/sdktests/testapi_hooks.go
@@ -84,23 +84,27 @@ func (h *Hooks) ExpectCall(t *ldtest.T, hookName string,
 	}
 }
 
-func (h *Hooks) ExpectSingleCallForEachHook(t *ldtest.T, hookNames []string, count int) []servicedef.HookExecutionPayload {
+// ExpectAtLeastOneCallForEachHook waits for a single call from N hooks. If there are fewer calls recorded,
+// the test will fail. However, this helper cannot detect if there were more calls waiting to be recorded.
+func (h *Hooks) ExpectAtLeastOneCallForEachHook(t *ldtest.T, hookNames []string) []servicedef.HookExecutionPayload {
 	out := make(chan o.Maybe[servicedef.HookExecutionPayload])
 
+	totalCalls := len(hookNames)
+
 	for _, hookName := range hookNames {
 		go func(name string) {
 			out <- helpers.TryReceive(h.instances[name].hookService.CallChannel, hookReceiveTimeout)
 		}(hookName)
 	}
 
-	payloads := make([]servicedef.HookExecutionPayload, 0)
-	for i := 0; i < count; i++ {
+	payloads := make([]servicedef.HookExecutionPayload, totalCalls)
+	for i := 0; i < totalCalls; i++ {
 		if val := <-out; val.IsDefined() {
 			payloads = append(payloads, val.Value())
 		}
 	}
 
-	assert.Len(t, payloads, count, "Expected %d hook calls, got %d", count, len(payloads))
+	assert.Len(t, payloads, totalCalls, "Expected %d hook calls, got %d", totalCalls, len(payloads))
 
 	return payloads
 }

From dcdb679cd02d30fcfa7c1409bc99b6c83e59c557 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 13:10:32 -0700
Subject: [PATCH 07/11] lint

---
 sdktests/server_side_hooks.go | 6 ++++--
 sdktests/testapi_hooks.go     | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/sdktests/server_side_hooks.go b/sdktests/server_side_hooks.go
index 425c1ad..f0029a8 100644
--- a/sdktests/server_side_hooks.go
+++ b/sdktests/server_side_hooks.go
@@ -1,10 +1,11 @@
 package sdktests
 
 import (
+	"strconv"
+
 	"github.com/launchdarkly/go-sdk-common/v3/ldcontext"
 	"github.com/launchdarkly/go-sdk-common/v3/ldmigration"
 	"github.com/launchdarkly/go-sdk-common/v3/ldvalue"
-	"strconv"
 
 	"github.com/launchdarkly/go-server-sdk-evaluation/v3/ldbuilders"
 
@@ -321,7 +322,8 @@ func createClientForHooks(t *ldtest.T, instances []string,
 }
 
 func createClientForHooksWithErrors(t *ldtest.T, instances []string,
-	hookData map[servicedef.HookStage]servicedef.SDKConfigEvaluationHookData, hookErrors map[servicedef.HookStage]o.Maybe[string]) (*SDKClient, *Hooks) {
+	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()
diff --git a/sdktests/testapi_hooks.go b/sdktests/testapi_hooks.go
index 82ae6ff..8f16e99 100644
--- a/sdktests/testapi_hooks.go
+++ b/sdktests/testapi_hooks.go
@@ -1,9 +1,10 @@
 package sdktests
 
 import (
-	"github.com/stretchr/testify/assert"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/launchdarkly/sdk-test-harness/v2/framework"
 	"github.com/launchdarkly/sdk-test-harness/v2/framework/harness"
 	"github.com/launchdarkly/sdk-test-harness/v2/framework/helpers"

From 2ee47c7efd2c96adf7e843754831df4972019ce4 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 15:19:38 -0700
Subject: [PATCH 08/11] don't use make() for slice

---
 sdktests/testapi_hooks.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sdktests/testapi_hooks.go b/sdktests/testapi_hooks.go
index 8f16e99..0b15399 100644
--- a/sdktests/testapi_hooks.go
+++ b/sdktests/testapi_hooks.go
@@ -98,7 +98,7 @@ func (h *Hooks) ExpectAtLeastOneCallForEachHook(t *ldtest.T, hookNames []string)
 		}(hookName)
 	}
 
-	payloads := make([]servicedef.HookExecutionPayload, totalCalls)
+	var payloads []servicedef.HookExecutionPayload
 	for i := 0; i < totalCalls; i++ {
 		if val := <-out; val.IsDefined() {
 			payloads = append(payloads, val.Value())

From 9c4531171cec488e26ab44bb70f1421c267a45e1 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 15:23:29 -0700
Subject: [PATCH 09/11] clarify docs

---
 docs/service_spec.md | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/service_spec.md b/docs/service_spec.md
index 89b9580..39b3964 100644
--- a/docs/service_spec.md
+++ b/docs/service_spec.md
@@ -194,9 +194,10 @@ 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. The error message itself is not tested by the framework at this time, as it is not a specified behavior.
+      * `errors` (object, optional): Specifies that an error should be returned/exception thrown from a stage. If present, this behavior should take place instead of returning any data specified in the `data` object.
+       The error message itself is not tested by the framework at this time, as it is not a specified behavior.
         * `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.
+        * `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).
 

From 89b58a37f46a7759722daa7fec3628d23e2a2e38 Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 15:23:37 -0700
Subject: [PATCH 10/11] only use 3 hooks, don't be insane

---
 sdktests/server_side_hooks.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sdktests/server_side_hooks.go b/sdktests/server_side_hooks.go
index f0029a8..e4b8e8c 100644
--- a/sdktests/server_side_hooks.go
+++ b/sdktests/server_side_hooks.go
@@ -276,7 +276,7 @@ func beforeEvaluationDataPropagatesToAfterMigration(t *ldtest.T) {
 // The client MUST handle exceptions which are thrown (or errors returned, if idiomatic for the language)
 // during the execution of a stage or handler allowing operations to complete unaffected.
 func errorInBeforeStageDoesNotAffectAfterStage(t *ldtest.T) {
-	const numHooks = 100 // why not?
+	const numHooks = 3
 
 	// 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
@@ -312,7 +312,7 @@ func errorInBeforeStageDoesNotAffectAfterStage(t *ldtest.T) {
 			"returned in this stage")
 
 		assert.Equal(t, 0, len(call.EvaluationSeriesData.Value()), "HOOKS:1.3.7.1: Since "+
-			"beforeEvaluation should have failed, the data passed to afterEvaluation should be an empty string")
+			"beforeEvaluation should have failed, the data passed to afterEvaluation should be empty")
 	}
 }
 

From f78380da95b60097d300eebd5c3b151556c7ffde Mon Sep 17 00:00:00 2001
From: Casey Waldren <cwaldren@launchdarkly.com>
Date: Tue, 2 Apr 2024 15:26:12 -0700
Subject: [PATCH 11/11] clarify test hook behavior

---
 docs/service_spec.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/service_spec.md b/docs/service_spec.md
index 39b3964..ee82297 100644
--- a/docs/service_spec.md
+++ b/docs/service_spec.md
@@ -132,7 +132,8 @@ For a test service to support hooks testing it must support a `test-hook`. The c
 
 A test hook must:
   - Implement the SDK hook interface.
-  - Whenever an evaluation stage is called post information about that call to the `callbackUrl` of the hook.
+  - Whenever an evaluation stage is called POST information about that call to the `callbackUrl` of the hook.
+  - The POST should not take place if the hook was configured to return/throw an error for that stage (`errors` object in the configuration).
     - The payload is an object with the following properties:
       * `evaluationSeriesContext` (object, optional): If an evaluation stage was executed, then this should be the associated context.
         * `flagKey` (string, required): The key of the flag being evaluated.