Re: [Tarantool-patches] [PATCH v4 2/4] http: remove redundant & incorrect case for curl_request code

[Alexander Turenko <alexander.turenko@tarantool.org>]

This commit should land before accept_encoding, because the feature
opens more possibilities to step into the problem that is fixed by this
patch.

Also I think the the fix should land to all 1.10+ branches: please,
verify that this will be done right when the patch will land.

The code LGTM. See several comments about texts below.

WBR, Alexander Turenko.

> http: remove redundant & incorrect case for curl_request code

Please, try to fit the commit header to 50 symbols if possible. Say
(just as example):

 | httpc: fix assertion fail after a write error

Is I remember it right: any CURLE_WRITE_ERROR did lead to this assertion
fail? Or only unknown Content-Encoding?

>
> After executing curl request we need to process curl_request code. It
> might be CURLE_WRITE_ERROR. We had special case for it, which assumed
> diagnostic message being set and contained corresponding assert, though
> it is incorrect. Better way is to handle it as any other non-standard
> event.

I would add an information how do you catched this: your story about
unknown encoding. This woudl answer to the question why the patch was
made: I think it is good property, when a reader can understood it from
a commit message even when (s)he it is not much involved into a project.

> 
> Part of #4232
> ---
>  src/httpc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/src/httpc.c b/src/httpc.c
> index 146a6f067..22b54d16a 100644
> --- a/src/httpc.c
> +++ b/src/httpc.c
> @@ -445,16 +445,12 @@ httpc_execute(struct httpc_request *req, double timeout)
>  	case CURLE_COULDNT_RESOLVE_PROXY:
>  	case CURLE_COULDNT_RESOLVE_HOST:
>  	case CURLE_COULDNT_CONNECT:
> +	case CURLE_WRITE_ERROR:
>  		/* 595 Connection Problem (AnyEvent non-standard) */
>  		req->status = 595;
>  		req->reason = curl_easy_strerror(req->curl_request.code);
>  		++env->stat.failed_requests;
>  		break;
> -	case CURLE_WRITE_ERROR:
> -		/* Diag is already set by curl_write_cb() */
> -		assert(!diag_is_empty(&fiber()->diag));
> -		++env->stat.failed_requests;
> -		return -1;
>  	case CURLE_OUT_OF_MEMORY:
>  		diag_set(OutOfMemory, 0, "curl", "internal");
>  		++env->stat.failed_requests;
> -- 
> 2.17.1
>