fix: split tests

internal/api/errorcodes.go

@@ -74,6 +74,5 @@ const (
 	ErrorCodeOverSMSSendRateLimit              ErrorCode = "over_sms_send_rate_limit"
 	ErrorBadCodeVerifier                       ErrorCode = "bad_code_verifier"
 	ErrorCodeAnonymousProviderDisabled         ErrorCode = "anonymous_provider_disabled"
-	//TODO: Find a better name for this
-	ErrorHookGatewayTimeout ErrorCode = "hook_gateway_timeout"
+	ErrorHookTimeout                           ErrorCode = "hook_timeout"
 )

internal/api/hooks.go

@@ -9,7 +9,6 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptrace"
-	"net/url"
 	"strings"
 	"time"
 
@@ -99,7 +98,6 @@ func readBodyWithLimit(rsp *http.Response) ([]byte, error) {
 }
 
 func (a *API) runHTTPHook(r *http.Request, hookConfig conf.ExtensibilityPointConfiguration, input, output any) ([]byte, error) {
-
 	client := http.Client{
 		Timeout: DefaultHTTPHookTimeout,
 	}
@@ -118,7 +116,7 @@ func (a *API) runHTTPHook(r *http.Request, hookConfig conf.ExtensibilityPointCon
 	for i := 0; i < DefaultHTTPHookRetries; i++ {
 		hookLog.Infof("invocation attempt: %d", i)
 		if time.Since(start) > time.Duration(i+1)*DefaultHTTPHookTimeout {
-			return []byte{}, gatewayTimeoutError(ErrorHookGatewayTimeout, "failed to reach hook within timeout")
+			return []byte{}, gatewayTimeoutError(ErrorHookTimeout, "failed to reach hook within timeout")
 		}
 		msgID := uuid.Must(uuid.NewV4())
 		currentTime := time.Now()
@@ -150,7 +148,7 @@ func (a *API) runHTTPHook(r *http.Request, hookConfig conf.ExtensibilityPointCon
 				time.Sleep(HTTPHookBackoffDuration)
 				continue
 			} else if i == DefaultHTTPHookRetries-1 {
-				return nil, gatewayTimeoutError(ErrorHookGatewayTimeout, "failed to reach hook within allotted interval")
+				return nil, gatewayTimeoutError(ErrorHookTimeout, "Failed to reach hook within allotted interval")
 
 			} else {
 				return nil, internalServerError("Failed to trigger auth hook, error making HTTP request").WithInternalError(err)
@@ -169,14 +167,15 @@ func (a *API) runHTTPHook(r *http.Request, hookConfig conf.ExtensibilityPointCon
 			return body, nil
 		case http.StatusTooManyRequests, http.StatusServiceUnavailable:
 			retryAfterHeader := rsp.Header.Get("retry-after")
+			// Check for truthy values to allow for flexibility to swtich to time duration
 			if retryAfterHeader != "" {
 				continue
 			}
-			return []byte{}, internalServerError("Service unavailable")
+			return []byte{}, internalServerError("Service currently unavailable")
 		case http.StatusBadRequest:
-			return nil, badRequestError(ErrorCodeValidationFailed, "Invalid Hook Payload")
+			return nil, badRequestError(ErrorCodeValidationFailed, "Invalid payload sent to hook")
 		case http.StatusUnauthorized:
-			return []byte{}, httpError(http.StatusUnauthorized, ErrorCodeNoAuthorization, "This endpoint requires a Bearer token")
+			return []byte{}, httpError(http.StatusUnauthorized, ErrorCodeNoAuthorization, "Hook requires authorizaition token")
 		default:
 			return []byte{}, internalServerError("Error executing Hook")
 		}
@@ -202,32 +201,7 @@ func (c *connectionWatcher) GotConn(_ httptrace.GotConnInfo) {
 	c.gotConn = true
 }
 
-func validateHTTPHook(uri string) error {
-	u, err := url.Parse(uri)
-	if err != nil {
-		return err
-	}
-	if !(strings.ToLower(u.Scheme) == "http" || strings.ToLower(u.Scheme) == "https") {
-		return fmt.Errorf("invalid http hook")
-	}
-	return nil
-}
-
-func validatePostgresHook(uri string) error {
-	u, err := url.Parse(uri)
-	if err != nil {
-		return err
-	}
-	if !(strings.ToLower(u.Scheme) == "pg-functions") {
-		return fmt.Errorf("invalid postgres hook")
-	}
-	return nil
-}
-
 func (a *API) invokeHTTPHook(r *http.Request, input, output any, hookURI string) error {
-	if err := validateHTTPHook(hookURI); err != nil {
-		return err
-	}
 	switch input.(type) {
 	case *hooks.CustomSMSProviderInput:
 		hookOutput, ok := output.(*hooks.CustomSMSProviderOutput)
@@ -244,7 +218,6 @@ func (a *API) invokeHTTPHook(r *http.Request, input, output any, hookURI string)
 			return err
 		}
 
-		// Check function logs for detailed hook error
 		if err := json.Unmarshal(response, hookOutput); err != nil {
 			return internalServerError("Error unmarshaling custom SMS provider hook output.").WithInternalError(err)
 		}
@@ -258,12 +231,9 @@ func (a *API) invokeHTTPHook(r *http.Request, input, output any, hookURI string)
 
 // invokePostgresHook invokes the hook code. tx can be nil, in which case a new
 // transaction is opened. If calling invokeHook within a transaction, always
-// pass the current transaciton, as pool-exhaustion deadlocks are very easy to
+// pass the current transaction, as pool-exhaustion deadlocks are very easy to
 // trigger.
 func (a *API) invokePostgresHook(ctx context.Context, conn *storage.Connection, input, output any, hookURI string) error {
-	if err := validatePostgresHook(hookURI); err != nil {
-		return err
-	}
 	config := a.config
 	// Switch based on hook type
 	switch input.(type) {

internal/api/hooks_test.go

@@ -60,7 +60,6 @@ func (ts *HooksTestSuite) TestRunHTTPHook() {
 		description  string
 		mockResponse interface{}
 		status       int
-		matchHeader  map[string]string
 		expectError  bool
 	}{
 		{
@@ -70,13 +69,6 @@ func (ts *HooksTestSuite) TestRunHTTPHook() {
 			expectError:  false,
 		},
 		{
-			description: "Too many requests with retry-after header",
-			status:      http.StatusGatewayTimeout,
-			matchHeader: map[string]string{"retry-after": "true"},
-			expectError: true,
-		},
-		{
-			// TODO: maybe properly check for a condition in the w/o retry case
 			description: "Too many requests without retry header should not retry",
 			status:      http.StatusGatewayTimeout,
 			expectError: true,
@@ -85,42 +77,71 @@ func (ts *HooksTestSuite) TestRunHTTPHook() {
 
 	for _, tc := range testCases {
 		ts.Run(tc.description, func() {
-			gock.Off()
-			request := gock.New(testURL).Post("/").MatchType("json").Reply(tc.status)
-
-			// Directly chain mock response setup
-			if tc.mockResponse != nil {
-				request.JSON(tc.mockResponse)
-			}
-
-			// If there are headers to match, especially for responses
-			if tc.status == http.StatusTooManyRequests && tc.matchHeader != nil {
-				for key, value := range tc.matchHeader {
-					// Instead of creating a new Gock interceptor,
-					// use the `request` variable to chain setting headers on the response.
-					request.SetHeader(key, value)
-				}
-			}
+			gock.New(ts.Config.Hook.CustomSMSProvider.URI).
+				Post("/").
+				MatchType("json").
+				Reply(tc.status).
+				JSON(tc.mockResponse) // gock handles nil gracefully.
 
 			var output hooks.CustomSMSProviderOutput
-
-			// Mock of original HTTP Rquest which triggered the hook
-			req, err := http.NewRequest("POST", "http://localhost:9998/otp", nil)
-			require.NoError(ts.T(), err)
-
+			req, _ := http.NewRequest("POST", ts.Config.Hook.CustomSMSProvider.URI, nil) // Simplified error handling.
 			body, err := ts.API.runHTTPHook(req, ts.Config.Hook.CustomSMSProvider, &input, &output)
-			if err == nil && body != nil {
-				err = json.Unmarshal(body, &output)
-				require.NoError(ts.T(), err, "Unmarshal should not fail")
-				require.True(ts.T(), output.Success, "Success should be true")
-			}
 
-			if tc.expectError {
-				require.Error(ts.T(), err)
-			} else {
+			if !tc.expectError {
 				require.NoError(ts.T(), err)
+				if body != nil {
+					require.NoError(ts.T(), json.Unmarshal(body, &output))
+					require.True(ts.T(), output.Success)
+				}
+			} else {
+				require.Error(ts.T(), err)
 			}
-			require.True(ts.T(), gock.IsDone(), "Expected HTTP mock to have been called")
+			require.True(ts.T(), gock.IsDone())
 		})
 	}
 }
+
+func (ts *HooksTestSuite) TestShouldRetryWithRetryAfterHeader() {
+	defer gock.OffAll()
+
+	input := hooks.CustomSMSProviderInput{
+		UserID: uuid.Must(uuid.NewV4()),
+		Phone:  "1234567890",
+		OTP:    "123456",
+	}
+	successOutput := hooks.CustomSMSProviderOutput{Success: true}
+	testURL := "http://localhost:54321/functions/v1/custom-sms-sender"
+	ts.Config.Hook.CustomSMSProvider.URI = testURL
+
+	gock.New(testURL).
+		Post("/").
+		MatchType("json").
+		Reply(http.StatusTooManyRequests).
+		SetHeader("retry-after", "true")
+
+	// Simulate an additional response for the retry attempt
+	gock.New(testURL).
+		Post("/").
+		MatchType("json").
+		Reply(http.StatusOK).
+		JSON(successOutput)
+
+	var output hooks.CustomSMSProviderOutput
+
+	// Simulate the original HTTP request which triggered the hook
+	req, err := http.NewRequest("POST", "http://localhost:9998/otp", nil)
+	require.NoError(ts.T(), err)
+
+	body, err := ts.API.runHTTPHook(req, ts.Config.Hook.CustomSMSProvider, &input, &output)
+	require.NoError(ts.T(), err)
+
+	// Assert that the retry was successful and the output is as expected
+	if body != nil {
+		err = json.Unmarshal(body, &output)
+		require.NoError(ts.T(), err, "Unmarshal should not fail")
+		require.True(ts.T(), output.Success, "Expected success on retry")
+	}
+
+	// Ensure that all expected HTTP interactions (mocks) have been called
+	require.True(ts.T(), gock.IsDone(), "Expected all mocks to have been called including retry")
+}

internal/conf/configuration.go

@@ -504,7 +504,7 @@ func (e *ExtensibilityPointConfiguration) ValidateExtensibilityPoint() error {
 	case "http":
 		hostname := u.Hostname()
 		if hostname == "localhost" || hostname == "127.0.0.1" || hostname == "::1" {
-			return validateHTTPSHookSecrets(e.HTTPHookSecrets)
+			return validateHTTPHookSecrets(e.HTTPHookSecrets)
 		}
 		return fmt.Errorf("only localhost, 127.0.0.1, and ::1 are supported with http")
 	case "https":