Handle all 2XX HTTP response status codes as success #324
Conversation
For HTTP target: Before - only 200 HTTP response code was treated as successful response. After - all 2XX are treated as successful response.
| @@ -219,7 +219,7 @@ func (ht *HTTPTarget) Write(messages []*models.Message) (*models.TargetWriteResu | |||
| continue | |||
| } | |||
| defer resp.Body.Close() | |||
| if resp.StatusCode == http.StatusOK { | |||
| if resp.StatusCode >= 200 && resp.StatusCode < 300 { | |||
There was a problem hiding this comment.
Should 3xx be treated as successful as well? I know in the Trackers a broad range of codes are seen as successful to ensure events do not get re-tried.
There was a problem hiding this comment.
There was a problem hiding this comment.
No, 3xx should not be treated as a success.
The go http client by default follows redirects, up to 10 times per request. So if we get a 3xx here then it means the server has issued 10 redirects in a row. That would be highly unexpected, it probably means the server is misconfigured, and it means our event has not landed safely in the destination.
There was a problem hiding this comment.
That's fair! Feels like we might need a follow-on ticket to think through how to handle other status code groups though as currently all 3xx, 4xx and 5xx will end up being retried forever.
It might not be worth retrying certain codes forever and certain codes might not be worth retrying at all (like if we hit 3xx requests with 10+ redirects - is it worth retrying that event to have that very expensive flow followed again and again?)...
There was a problem hiding this comment.
@jbeemster yeah agree - it's beyond the scope of this ticket (which is just a quick fix for a customer issue), but the intention is to make this configurable, so that behaviour can be tailored to what's best suited to a given target.
I'm hoping to flesh this out in a design doc alongside other supportability work. :)
For HTTP target: