Treat errors in templating or jsonifying requests as invalid

pkg/target/http.go

@@ -302,6 +302,7 @@ func (ht *HTTPTarget) Write(messages []*models.Message) (*models.TargetWriteResu
 
 	sent := []*models.Message{}
 	failed := []*models.Message{}
+	invalid := []*models.Message{}
 	var errResult error
 
 	for _, chunk := range chunks {
@@ -314,16 +315,13 @@ func (ht *HTTPTarget) Write(messages []*models.Message) (*models.TargetWriteResu
 			var err error
 
 			if ht.requestTemplate != nil {
-				reqBody, goodMsgs, badMsgs, err = ht.renderBatchUsingTemplate(group)
+				reqBody, goodMsgs, badMsgs = ht.renderBatchUsingTemplate(group)
 			} else {
-				reqBody, goodMsgs, badMsgs, err = ht.provideRequestBody(group)
+				reqBody, goodMsgs, badMsgs = ht.provideRequestBody(group)
 			}
 
-			failed = append(failed, badMsgs...)
-			if err != nil {
-				errResult = multierror.Append(errResult, errors.New("Error constructing request"))
-				continue
-			}
+			invalid = append(invalid, badMsgs...)
+
 			if len(goodMsgs) == 0 {
 				continue
 			}
@@ -406,9 +404,7 @@ func (ht *HTTPTarget) retrieveHeaders(msg *models.Message) map[string]string {
 }
 
 // renderBatchUsingTemplate creates a request from a batch of messages based on configured template
-func (ht *HTTPTarget) renderBatchUsingTemplate(messages []*models.Message) (templated []byte, success []*models.Message, failed []*models.Message, err error) {
-	invalid := make([]*models.Message, 0)
-	validMsgs := make([]*models.Message, 0)
+func (ht *HTTPTarget) renderBatchUsingTemplate(messages []*models.Message) (templated []byte, success []*models.Message, invalid []*models.Message) {
 	validJsons := []interface{}{}
 
 	for _, msg := range messages {
@@ -420,22 +416,26 @@ func (ht *HTTPTarget) renderBatchUsingTemplate(messages []*models.Message) (temp
 			continue
 		}
 
-		validMsgs = append(validMsgs, msg)
+		success = append(success, msg)
 		validJsons = append(validJsons, asJSON)
 	}
 
 	var buf bytes.Buffer
-	if err := ht.requestTemplate.Execute(&buf, validJsons); err != nil {
-		invalid = append(invalid, validMsgs...)
-		return nil, nil, invalid, err
+	tmplErr := ht.requestTemplate.Execute(&buf, validJsons)
+	if tmplErr != nil {
+		for _, msg := range success {
+			msg.SetError(errors.Wrap(tmplErr, "Could not create request JSON"))
+			invalid = append(invalid, msg)
+		}
+		return nil, nil, invalid
 	}
 
-	return buf.Bytes(), validMsgs, invalid, nil
+	return buf.Bytes(), success, invalid
 }
 
 // Where no transformation function provides a request body, we must provide one - this necessarily must happen last.
 // This is a http specific function so we define it here to avoid scope for misconfiguration
-func (ht *HTTPTarget) provideRequestBody(messages []*models.Message) (templated []byte, success []*models.Message, invalid []*models.Message, err error) {
+func (ht *HTTPTarget) provideRequestBody(messages []*models.Message) (templated []byte, success []*models.Message, invalid []*models.Message) { // TODO: REMOVE RETURNING ERROR FROM BOTH
 
 	// This assumes the data is a valid JSON. Plain strings are no longer supported, but can be handled via a combination of transformation and templater
 	requestData := make([]json.RawMessage, 0)
@@ -449,15 +449,19 @@ func (ht *HTTPTarget) provideRequestBody(messages []*models.Message) (templated
 		}
 
 		requestData = append(requestData, msg.Data)
+		success = append(success, msg)
 	}
 
 	requestBody, err := json.Marshal(requestData)
 	if err != nil {
-		// Since we have checked that each message is valid JSON above, an error here means something unexpected is wrong and so we treat it as such
-		return nil, nil, messages, err
+		for _, msg := range success {
+			msg.SetError(errors.Wrap(err, "Could not create request JSON"))
+			invalid = append(invalid, msg)
+		}
+		return nil, nil, invalid
 	}
 
-	return requestBody, messages, invalid, nil
+	return requestBody, messages, invalid
 }
 
 // groupByDynamicHeaders batches data by header if the dynamic header feature is turned on.

pkg/target/http_templating_test.go

@@ -37,8 +37,7 @@ func TestTemplating_WithPrettyPrint(t *testing.T) {
 		{Data: []byte(`{ "event_data": { "nested": "value3"}, "attribute_data": 3}`)},
 	}
 
-	templated, goodMessages, invalidMessages, err := target.renderBatchUsingTemplate(inputMessages)
-	assert.Nil(err)
+	templated, goodMessages, invalidMessages := target.renderBatchUsingTemplate(inputMessages)
 
 	expectedOutput := "{\n  \"attributes\": [1,2,3],\n  \"events\": [{\"nested\":\"value1\"},{\"nested\":\"value2\"},{\"nested\":\"value3\"}]\n}"
 	assert.Equal(expectedOutput, string(templated))
@@ -65,8 +64,7 @@ func TestTemplating_NoPrettyPrinting(t *testing.T) {
 		{Data: []byte(`{ "event_data": { "nested": "value3"}, "attribute_data": 3}`)},
 	}
 
-	templated, goodMessages, invalidMessages, err := target.renderBatchUsingTemplate(inputMessages)
-	assert.Nil(err)
+	templated, goodMessages, invalidMessages := target.renderBatchUsingTemplate(inputMessages)
 
 	//we get a stringified map for JSON
 	expectedOutput := "{\n  \"attributes\": [1,2,3],\n  \"events\": [map[nested:value1],map[nested:value2],map[nested:value3]]\n}"
@@ -91,8 +89,7 @@ func TestTemplating_ArrayProvided(t *testing.T) {
 		{Data: []byte(`["value1", "value2", "value3"]`)},
 	}
 
-	templated, goodMessages, invalidMessages, err := target.renderBatchUsingTemplate(inputMessages)
-	assert.Nil(err)
+	templated, goodMessages, invalidMessages := target.renderBatchUsingTemplate(inputMessages)
 
 	//we get a stringified map for JSON
 	expectedOutput := "{\n  \"attributes\": [\"Value: value1\",\"Value: value2\",\"Value: value3\"]\n}"
@@ -128,8 +125,7 @@ func TestTemplating_AccessNonExistingField(t *testing.T) {
 
 			inputMessages := []*models.Message{{Data: []byte(tt.Input)}}
 
-			templated, goodMessages, invalidMessages, err := target.renderBatchUsingTemplate(inputMessages)
-			assert.Nil(err)
+			templated, goodMessages, invalidMessages := target.renderBatchUsingTemplate(inputMessages)
 
 			assert.Equal(tt.Output, string(templated))
 			assert.Equal(inputMessages, goodMessages)
@@ -161,8 +157,7 @@ func TestTemplating_JSONParseFailure(t *testing.T) {
 		{Data: []byte(`plain string, can't unmarshall`)},
 	}
 
-	templated, goodMessages, invalidMessages, err := target.renderBatchUsingTemplate(inputMessages)
-	assert.Nil(err)
+	templated, goodMessages, invalidMessages := target.renderBatchUsingTemplate(inputMessages)
 
 	assert.Equal("{\"nested\":\"value1\"}", string(templated))
 
@@ -188,10 +183,10 @@ func TestTemplating_RenderFailure(t *testing.T) {
 		{Data: []byte(`{ "event_data": { "nested": "value1"}, "attribute_data": 1}`)},
 	}
 
-	templated, goodMessages, invalidMessages, err := target.renderBatchUsingTemplate(inputMessages)
+	templated, goodMessages, invalidMessages := target.renderBatchUsingTemplate(inputMessages)
 
-	expectedError := `template: HTTP:1:3: executing "HTTP" at <index . 1>: error calling index: reflect: slice index out of range`
-	assert.Equal(expectedError, err.Error())
+	expectedError := `Could not create request JSON: template: HTTP:1:3: executing "HTTP" at <index . 1>: error calling index: reflect: slice index out of range`
+	assert.Equal(expectedError, invalidMessages[0].GetError().Error())
 	assert.Empty(templated)
 	assert.Empty(goodMessages)
 	assert.Equal(inputMessages, invalidMessages)