Gracefully handles client request cancelations, instead of treating treating them as server errors (#1194)

* Gracefully handles client request cancelations, instead of logging them as a 500 error

* adds runner_addr to runner client logs
This commit is contained in:
Gerardo Viedma
2018-09-05 07:53:48 +01:00
committed by GitHub
parent b11c1e63a1
commit 0e01f3e547
2 changed files with 18 additions and 8 deletions

View File

@@ -192,8 +192,8 @@ func (r *gRPCRunner) TryExec(ctx context.Context, call pool.RunnerCall) (bool, e
recvDone := make(chan error, 1)
go receiveFromRunner(ctx, runnerConnection, call, recvDone)
go sendToRunner(ctx, runnerConnection, call)
go receiveFromRunner(ctx, runnerConnection, r.address, call, recvDone)
go sendToRunner(ctx, runnerConnection, r.address, call)
select {
case <-ctx.Done():
@@ -208,11 +208,11 @@ func (r *gRPCRunner) TryExec(ctx context.Context, call pool.RunnerCall) (bool, e
}
}
func sendToRunner(ctx context.Context, protocolClient pb.RunnerProtocol_EngageClient, call pool.RunnerCall) {
func sendToRunner(ctx context.Context, protocolClient pb.RunnerProtocol_EngageClient, runnerAddress string, call pool.RunnerCall) {
bodyReader := call.RequestBody()
writeBuffer := make([]byte, MaxDataChunk)
log := common.Logger(ctx)
log := common.Logger(ctx).WithField("runner_addr", runnerAddress)
// IMPORTANT: IO Read below can fail in multiple go-routine cases (in retry
// case especially if receiveFromRunner go-routine receives a NACK while sendToRunner is
// already blocked on a read) or in the case of reading the http body multiple times (retries.)
@@ -297,11 +297,11 @@ func recordFinishStats(ctx context.Context, msg *pb.CallFinished) {
}
}
func receiveFromRunner(ctx context.Context, protocolClient pb.RunnerProtocol_EngageClient, c pool.RunnerCall, done chan error) {
func receiveFromRunner(ctx context.Context, protocolClient pb.RunnerProtocol_EngageClient, runnerAddress string, c pool.RunnerCall, done chan error) {
w := c.ResponseWriter()
defer close(done)
log := common.Logger(ctx)
log := common.Logger(ctx).WithField("runner_addr", runnerAddress)
isPartialWrite := false
DataLoop:
@@ -358,7 +358,7 @@ DataLoop:
break DataLoop
default:
log.Error("Ignoring unknown message type %T from runner, possible client/server mismatch", body)
log.Errorf("Ignoring unknown message type %T from runner, possible client/server mismatch", body)
}
}

View File

@@ -28,6 +28,11 @@ func simpleError(err error) *models.Error {
// TODO delete me !
func handleV1ErrorResponse(ctx *gin.Context, err error) {
log := common.Logger(ctx)
if ctx.Request.Context().Err() == context.Canceled {
log.Info("request canceled")
return
}
w := ctx.Writer
var statuscode int
if e, ok := err.(models.APIError); ok {
@@ -63,9 +68,14 @@ func writeV1Error(ctx context.Context, w http.ResponseWriter, statuscode int, er
}
}
// handleV1ErrorResponse used to handle response errors in the same way.
// HandleErrorResponse used to handle response errors in the same way.
func HandleErrorResponse(ctx context.Context, w http.ResponseWriter, err error) {
log := common.Logger(ctx)
if ctx.Err() == context.Canceled {
log.Info("request canceled")
return
}
var statuscode int
if e, ok := err.(models.APIError); ok {
if e.Code() >= 500 {