[query] Handle context.Canceled and map to 4xx http status #3069
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3069 +/- ##
=======================================
Coverage 72.2% 72.2%
=======================================
Files 1084 1084
Lines 100199 100201 +2
=======================================
+ Hits 72346 72399 +53
+ Misses 22797 22761 -36
+ Partials 5056 5041 -15
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
src/x/net/http/errors.go
Outdated
| @@ -132,7 +132,7 @@ func getStatusCode(err error) int { | |||
| case Error: | |||
| return v.Code() | |||
| case error: | |||
| if xerrors.IsInvalidParams(v) { | |||
| if xerrors.IsInvalidParams(v) || errors.Is(err, context.Canceled) { | |||
There was a problem hiding this comment.
I believe this should be http.StatusGatewayTimeout since this is the same as the context deadline being exceeded, essentially a timeout.
There was a problem hiding this comment.
Hmm well StatusGatewayTimeout is 504, but contextCanceled means client canceled the request. It could happen because request took too long and client canceled. For example Nginx came up with their own non 500 status code for this case https://httpstatuses.com/499
I thought smth from 4xx range would be better then 5xx in this case. Since client just abandoned the request which did not reach specified deadline.
Antanukas
force-pushed
the
antanas/ch5674/handle-context-cancel
branch
from
5b489e8
to
ad387bf
Compare
Antanukas
force-pushed
the
antanas/ch5674/handle-context-cancel
branch
from
aee8aab
to
9c0d605
Compare
* master: [dbnode] Add aggregate term limit regression test (#3135) [DOCS] Adding Prometheus steps to quickstart (#3043) [dbnode] Revert AggregateQuery changes (#3133) Fix TestSessionFetchIDs flaky test (#3132) [dbnode] Alter multi-segments builder to order by size before processing (#3128) [dbnode] Emit aggregate usage metrics (#3123) [dbnode] Add Shard.OpenStreamingReader method (#3119) [dtests] Docker tests integration with docker-compose (#3031) [dbnode] Comments / remove unused var (#3124) [query] Handle context.Canceled and map to 499 http status (#3069) [dbnode] Use StreamingReadMetadata for bootstrapping (#2938) [dbnode] Use DefaultTestOptions in test code (#3113) # Conflicts: # src/dbnode/storage/bootstrap/bootstrapper/fs/source.go
What this PR does / why we need it:
Handles context.Canceled on query side.
Special notes for your reviewer:
NONE
Does this PR introduce a user-facing and/or backwards incompatible change?:
context.Cancel should usually happen when the client is not expecting response anymore so this should not be a breaking change.
Does this PR require updating code package or user-facing documentation?: