Elasticsearch: Server-side timeout returns 408 Request Timeout
In a few places we return RestStatus.REQUEST_TIMEOUT (i.e. 408 Request Timeout) when a request times out. However, RFC 7231 says:
The 408 (Request Timeout) status code indicates that the server did
not receive a complete request message within the time that it was
prepared to wait. A server SHOULD send the "close" connection option
(Section 6.1 of [RFC7230]) in the response, since 408 implies that
the server has decided to close the connection rather than continue
waiting. If the client has an outstanding request in transit, the
client MAY repeat that request on a new connection.
I think this does not describe the usage in the following places, all of which seem to be server-side timeouts rather than timeouts waiting for another request from the client:
DaveCTurner
All 6 comments
Pinging @elastic/es-core-infra
elasticmachine
on 15 Nov 2018
We discussed this and, although we agreed that 408 isn't the best choice we didn't agree on an alternative. The impact on clients will be significant. Perhaps we can keep these in the 4xx series, although the most appropriate codes seem to be a general 400 or else perhaps 409 Conflict?
@elastic/es-clients do you have any opinions on a way forwards here?
DaveCTurner
on 16 Nov 2018
Since it is a server timeout I'd say that 504 would be better than 4xx response as that indicates a client error. We just need to be careful because 504 is a retryable error and the client will, by default, attempt to retry on a different node.
Either way it is important to have a documented and stable information in the response body indicating exactly what kind of error it is, then the client code doesn't have to rely solely on the status code which has limited options.
HonzaKral
on 16 Nov 2018
We chatted about this again given @HonzaKral's feedback, and settled on just a generic 500 error.
We agreed with the rationale that this should be a 5xx level error, but didn't want to use a 504 because that implies Elasticsearch is acting as a proxy. ES should never be in the position to be a proxy/gateway, so we should probably never send a 504 anywhere. None of the other 5xx seemed to make sense in this situation, so that left us with a generic 500
polyfractal
on 30 Nov 2018
ES should never be in the position to be a proxy/gateway, so we should probably never send a 504 anywhere.
This may have been true in the past but with cross cluster search and remote clusters, I wonder if we should reconsider the 504 as the local node may simply act as a gateway to multiple remotes? Some other items worth discussion should include when we make such a change, will our API compatibility solution be able to support making this and avoid breaking clients?
jaymode
on 14 Dec 2020
I disagree that this should be a 5xx error. The client specified the timeout, and if the only bad thing that happened to the request was that Elasticsearch didn't get to it before it the timeout elapsed then that's kinda the client's problem. Note that I'm distinguishing this case from ones where the request was actually rejected (429 overload) or genuinely failed (5xx our bad)
IMO a 504 is appropriate where the timeout is defined by the gateway, but that doesn't apply to Elasticsearch.
DaveCTurner
on 14 Dec 2020
Related issues
jasontedor
路
3Comments
abtpst
路
3Comments
martijnvg
路
3Comments
brwe
路
3Comments
malpani
路
3Comments
Most helpful comment
We chatted about this again given @HonzaKral's feedback, and settled on just a generic
500error.We agreed with the rationale that this should be a 5xx level error, but didn't want to use a
504because that implies Elasticsearch is acting as a proxy. ES should never be in the position to be a proxy/gateway, so we should probably never send a 504 anywhere. None of the other 5xx seemed to make sense in this situation, so that left us with a generic500