aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorhor911 <hor911@ydb.tech>2023-02-21 16:08:48 +0300
committerhor911 <hor911@ydb.tech>2023-02-21 16:08:48 +0300
commita592760719385100032c595377aee9cd5c6b3977 (patch)
treef4996c076dea5c68ded0b36126562e515c29a429
parentd203996858cdf8c2c59024e6cedab2819d17ebe5 (diff)
downloadydb-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.cpp23
-rw-r--r--ydb/library/yql/providers/s3/actors/yql_s3_read_actor.cpp8
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