diff options
author | hor911 <hor911@ydb.tech> | 2023-02-21 16:08:48 +0300 |
---|---|---|
committer | hor911 <hor911@ydb.tech> | 2023-02-21 16:08:48 +0300 |
commit | a592760719385100032c595377aee9cd5c6b3977 (patch) | |
tree | f4996c076dea5c68ded0b36126562e515c29a429 | |
parent | d203996858cdf8c2c59024e6cedab2819d17ebe5 (diff) | |
download | ydb-a592760719385100032c595377aee9cd5c6b3977.tar.gz |
Error handling when Content-Length == 0
Фикс нетривиальный. Он чинит несколько проблем:
1. Ситуация когда у нас возвращается код ошибки (i.e. 403) и пустой body (Content-Length: 0). В это случае не вызывался TEasyCurlStream::Write(), не заполнялось поле HttpResponseCode и не вызывался callback OnStart(), как следствие в корректор не приходило событие TEvPrivate::TEvReadStarted и не работал retry policy.
2. Для ретраев в рамках https://a.yandex-team.ru/review/2940722 было добавлен код анализа retry police в обработчик TEvReadFinished, но если OnStart() все-таки вызывается (а теперь он вызывается всегда) это генерировало двойной вызов GetNextRetryDelay() что некорректно и может даже бесконечно перезапускать запросы в некоторых случаях, убираю это.
3. Аналогично в отсутствие данных не срабатывало событие TEvDataPart и не возвращался код ошибки HTTP в Issues, поправил и это тоже.
Как это проверить-зафиксировать тестом - не знаю. Надо уметь возвращать управляемые ошибки от эмулятора S3 MOTO, но там такой функциональности нет. Собственно с ошибкой я столкнулся, когда отлаживал на этом эмуляторе Multi-part Upload, а он не работал из-за ошибок с обоих сторон.
-rw-r--r-- | ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp | 23 | ||||
-rw-r--r-- | ydb/library/yql/providers/s3/actors/yql_s3_read_actor.cpp | 8 |
2 files changed, 22 insertions, 9 deletions
diff --git a/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp b/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp index 4926eaa37b..95b26e9219 100644 --- a/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp +++ b/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp @@ -411,20 +411,27 @@ private: OnFinish(TIssues{error}); } - void Done(CURLcode result, long) final { - if (CURLE_OK != result) - return Fail(TIssue(TStringBuilder{} << "error: " << curl_easy_strerror(result) << " detailed: " << GetDetailedErrorText())); - - if (!Cancelled) - OnFinish(TIssues()); + void MaybeStart() { + if (!HttpResponseCode) { + curl_easy_getinfo(GetHandle(), CURLINFO_RESPONSE_CODE, &HttpResponseCode); + OnStart(HttpResponseCode); + } } - size_t Write(void* contents, size_t size, size_t nmemb) final { + void Done(CURLcode result, long httpResponseCode) final { + if (CURLE_OK != result) { + return Fail(TIssue(TStringBuilder{} << "error: " << curl_easy_strerror(result) << " detailed: " << GetDetailedErrorText())); + } if (!HttpResponseCode) { - curl_easy_getinfo(GetHandle(), CURLINFO_RESPONSE_CODE, &HttpResponseCode); + HttpResponseCode = httpResponseCode; OnStart(HttpResponseCode); } + if (!Cancelled) + OnFinish(TIssues()); + } + size_t Write(void* contents, size_t size, size_t nmemb) final { + MaybeStart(); const auto realsize = size * nmemb; if (!Cancelled) OnNewData(IHTTPGateway::TCountedContent(TString(static_cast<char*>(contents), realsize), Counter, InflightCounter)); diff --git a/ydb/library/yql/providers/s3/actors/yql_s3_read_actor.cpp b/ydb/library/yql/providers/s3/actors/yql_s3_read_actor.cpp index 2c10403ba7..101ee0fee5 100644 --- a/ydb/library/yql/providers/s3/actors/yql_s3_read_actor.cpp +++ b/ydb/library/yql/providers/s3/actors/yql_s3_read_actor.cpp @@ -779,9 +779,15 @@ public: return true; case TEvPrivate::TEvReadFinished::EventType: Issues = std::move(ev->Get<TEvPrivate::TEvReadFinished>()->Issues); + + if (HttpResponseCode >= 300) { + ServerReturnedError = true; + Issues.AddIssue(TIssue{TStringBuilder() << "HTTP error code: " << HttpResponseCode}); + } + if (Issues) { LOG_CORO_D("TS3ReadCoroImpl", "TEvReadFinished. Url: " << RetryStuff->Url << ". Issues: " << Issues.ToOneLineString()); - if (RetryStuff->NextRetryDelay = RetryStuff->GetRetryState()->GetNextRetryDelay(0L); !RetryStuff->NextRetryDelay) { + if (!RetryStuff->NextRetryDelay) { InputFinished = true; LOG_CORO_W("TS3ReadCoroImpl", "ReadError: " << Issues.ToOneLineString() << ", Url: " << RetryStuff->Url << ", Offset: " << RetryStuff->Offset << ", LastOffset: " << LastOffset << ", LastData: " << GetLastDataAsText() << ", request id: [" << RetryStuff->RequestId << "]"); throw TS3ReadError(); // Don't pass control to data parsing, because it may validate eof and show wrong issues about incorrect data format |