diff --git a/amplitude/plugins/destination/internal/amplitude_http_client.go b/amplitude/plugins/destination/internal/amplitude_http_client.go index 1767a39..60a7c71 100644 --- a/amplitude/plugins/destination/internal/amplitude_http_client.go +++ b/amplitude/plugins/destination/internal/amplitude_http_client.go @@ -111,7 +111,6 @@ func (c *amplitudeHTTPClient) Send(payload AmplitudePayload) AmplitudeResponse { _ = json.Unmarshal(body, &litudeResponse) } else { c.logger.Debugf("HTTP response body is not valid JSON: %s", string(body)) - amplitudeResponse.Code = response.StatusCode } amplitudeResponse.Status = response.StatusCode diff --git a/amplitude/plugins/destination/internal/amplitude_http_client_test.go b/amplitude/plugins/destination/internal/amplitude_http_client_test.go index 7590134..fcc2ead 100644 --- a/amplitude/plugins/destination/internal/amplitude_http_client_test.go +++ b/amplitude/plugins/destination/internal/amplitude_http_client_test.go @@ -122,7 +122,7 @@ func (t *AmplitudeHTTPClientSuiteSuite) TestSend_Empty() { func (t *AmplitudeHTTPClientSuiteSuite) TestSend_Timeout() { timeout := time.Millisecond * 100 - server := t.createTestServer(timeout * 2, 200, `{"code": 234, "error": "some server error"}`) + server := t.createTestServer(timeout*2, 200, `{"code": 234, "error": "some server error"}`) client := internal.NewAmplitudeHTTPClient( server.URL, @@ -172,7 +172,9 @@ func (t *AmplitudeHTTPClientSuiteSuite) TestSend_NonJsonResponse() { t.Require().Equal(internal.AmplitudeResponse{ Status: 413, - Code: 413, + // amplitude_http_client always returns Code:0 + // if response body cannot be parsed (not json). + Code: 0, }, response) server.Close() diff --git a/amplitude/plugins/destination/internal/amplitude_response.go b/amplitude/plugins/destination/internal/amplitude_response.go index bca8832..d7e293c 100644 --- a/amplitude/plugins/destination/internal/amplitude_response.go +++ b/amplitude/plugins/destination/internal/amplitude_response.go @@ -7,10 +7,14 @@ import ( ) type AmplitudeResponse struct { - Status int `json:"-"` - Err error `json:"-"` - - Code int `json:"code"` + // An HTTP Response Code + Status int `json:"-"` + // An HTTP Response Err + Err error `json:"-"` + + // Code from the Response Body json + Code int `json:"code"` + // Error from the Response Body json Error string `json:"error"` MissingField string `json:"missing_field"` diff --git a/amplitude/plugins/destination/internal/amplitude_response_processor.go b/amplitude/plugins/destination/internal/amplitude_response_processor.go index fe9c8d2..3cef1dc 100644 --- a/amplitude/plugins/destination/internal/amplitude_response_processor.go +++ b/amplitude/plugins/destination/internal/amplitude_response_processor.go @@ -44,6 +44,10 @@ type amplitudeResponseProcessor struct { func (p *amplitudeResponseProcessor) Process(events []*types.StorageEvent, response AmplitudeResponse) AmplitudeProcessorResult { responseStatus := response.normalizedStatus() + if response.Code == 0 { + response.Code = responseStatus + } + var urlErr *url.Error isURLErr := errors.As(response.Err, &urlErr) diff --git a/amplitude/plugins/destination/internal/amplitude_response_processor_test.go b/amplitude/plugins/destination/internal/amplitude_response_processor_test.go index 6c1deec..b9cc42b 100644 --- a/amplitude/plugins/destination/internal/amplitude_response_processor_test.go +++ b/amplitude/plugins/destination/internal/amplitude_response_processor_test.go @@ -333,6 +333,83 @@ func (t *AmplitudeResponseProcessorSuite) TestProcessUnknownError_ResponseError( require.Equal(0, len(result.EventsForRetry)) } +func (t *AmplitudeResponseProcessorSuite) Test_Process_Overrides_Code_Eq_0() { + testCases := []struct { + name string + httpStatusCode int + expectCode int + }{ + { + name: "StatusCode:200 - success", + httpStatusCode: 200, + expectCode: 200, + }, + { + name: "StatusCode:299 - success", + httpStatusCode: 299, + expectCode: 200, + }, + { + name: "StatusCode:100 - unexpected, returns -1", + httpStatusCode: 100, + expectCode: -1, + }, + { + name: "StatusCode:429 - too many requests", + httpStatusCode: http.StatusTooManyRequests, + expectCode: 429, + }, + { + name: "StatusCode:413 - request entity too large", + httpStatusCode: http.StatusRequestEntityTooLarge, + expectCode: 413, + }, + { + name: "StatusCode:408 - request timeout", + httpStatusCode: http.StatusRequestTimeout, + expectCode: 408, + }, + { + name: "StatusCode:400 - bad request", + httpStatusCode: http.StatusBadRequest, + expectCode: 400, + }, + { + name: "StatusCode:418 - tea pot (and other unhandled 4xx)", + httpStatusCode: http.StatusTeapot, + expectCode: 400, + }, + { + name: "StatusCode:500 - internal server error", + httpStatusCode: http.StatusInternalServerError, + expectCode: 500, + }, + { + name: "StatusCode:502 - bad gateway (and other unhandled 5xx)", + httpStatusCode: http.StatusBadGateway, + expectCode: 500, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func() { + events := t.cloneOriginalEvents() + p := internal.NewAmplitudeResponseProcessor(internal.AmplitudeResponseProcessorOptions{ + Now: time.Now, + Logger: noopLogger{}, + }) + + result := p.Process(events, internal.AmplitudeResponse{ + Status: tt.httpStatusCode, + // processor.Process must override Code=0 + // with a normalized StatusCode + Code: 0, + }) + t.Require().Equal(tt.expectCode, result.Code) + }) + } +} + func (t *AmplitudeResponseProcessorSuite) cloneOriginalEvents() []*types.StorageEvent { events := make([]*types.StorageEvent, len(originalEvents))