Refine THProviderErrorMapper error mapping and MDC tests#78
Merged
karle0wne merged 1 commit intoepic/trace-enhancemetsfrom Oct 10, 2025
Merged
Refine THProviderErrorMapper error mapping and MDC tests#78karle0wne merged 1 commit intoepic/trace-enhancemetsfrom
karle0wne merged 1 commit intoepic/trace-enhancemetsfrom
Conversation
- extend MDC utils and interceptor logic, adding MdcUtilsExtendedTest coverage - align THProviderErrorMapper errorSource/generationSource rules and normalize HTTP messages using EnglishReasonPhraseCatalog - add dedicated THProviderErrorMapper tests and adjust thrift test suite expectations - declare missing httpcore5/httpclient5 dependencies
echerniak
approved these changes
Oct 10, 2025
karle0wne
added a commit
that referenced
this pull request
Oct 31, 2025
* init plan * guarantee MDC cleanup plus span termination when traces are absent (#73) * bump test log level * Update dependency org.apache.commons:commons-lang3 to v3.18.0 [SECURITY] (#70) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Replace dependency org.slf4j:slf4j-log4j12 with org.slf4j:slf4j-reload4j (#67) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update all non-major maven dependencies (#51) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * add missing pom sonatype elements * Refine THProviderErrorMapper error mapping and MDC tests (#78) - extend MDC utils and interceptor logic, adding MdcUtilsExtendedTest coverage - align THProviderErrorMapper errorSource/generationSource rules and normalize HTTP messages using EnglishReasonPhraseCatalog - add dedicated THProviderErrorMapper tests and adjust thrift test suite expectations - declare missing httpcore5/httpclient5 dependencies * refactor: unify trace metadata and rpc handling and add MDC propagation tests (#79) * feat: align OTel trace propagation across transport bundles and asynс flows (#80) * fixes * codacy * codacy * codacy * codacy * codacy * add otel attributes (#81) * second put mdc after filling metadata (#82) * add MdcRefreshInterceptor (#83) * bump * bump * bump --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
существующие тесты пропустили
для ускорения дебага // @test todo
Details
⛬ Как сейчас работает
THProviderErrorMapper(по шагам)THProviderErrorMapper — это центральный «переводчик» между внутренними исключениями/метаданными Woody и тем, что уходит
наружу: либо как HTTP-ответ (через THResponseInfo), либо как WErrorDefinition, который дальше разбирают слушатели и клиенты.
──────────────────────────────────────────
WErrorDefinition(например, в метаданных)getResponseInfo(ContextSpan span)• BUSINESS_ERROR → всегда 200, errClass = "business_error".
• PROVIDER_ERROR:
• Если это ошибка ещё на входе (запрос не успели обработать) → статус 4xx в зависимости от TErrorType/метаданных:
• PROTOCOL → 400
• TRANSPORT → mapTransportErrorStatus (405 / 415 / 400)
• Остальное → 400
• Если это уже в ответе → 500, если генерили сами (internal), или 502, если проксировали внешний сбой.
• UNAVAILABLE_RESULT → 503 (internal) или 502 (external).
• UNDEFINED_RESULT → 504 (internal) или 502 (external).
• UNEXPECTED_ERROR → 500 (internal) или 502 (external).
перебиваются на транспортный (400/405/415), потому что до сервиса мы не дошли.
──────────────────────────────────────────
WErrorDefinitionпо HTTP-ответуcreateErrorDefinition(THResponseInfo info, Supplier invalidErrClass)• 200 + errClass=business_error → создаём WErrorDefinition:
• generationSource = EXTERNAL
• errorType = BUSINESS_ERROR
• errorSource = INTERNAL (ошибка бизнес-логики сервиса)
• 503 → UNAVAILABLE_RESULT, generationSource = EXTERNAL, errorSource = INTERNAL.
• 504 → UNDEFINED_RESULT, generationSource = EXTERNAL, errorSource = INTERNAL.
• 502 → UNEXPECTED_ERROR (если не указано лучше), и обе стороны EXTERNAL. Если внутри 502 всё-таки сидит business_error,
вызываем invalidErrClass.get() — сигнал, что кто-то неправильно вернул бизнес-ошибку через 502.
• Любой другой статус (например, 400/500) → UNEXPECTED_ERROR, generationSource = EXTERNAL, errorSource = INTERNAL.
• Для 4xx ставим стандартные текстовые Reason-Phrase (Bad Request, Method Not Allowed, т. п.).
• Если у 5xx пустой errClass и есть errReason, чтобы не светить 500 без содержания, понижаем сообщение до «Bad Request».
• Если совсем ничего нет, используем дефолтный Reason-Phrase по статусу.
──────────────────────────────────────────
mapToDef(Throwable t, ContextSpan span)переписывать то, что поставили расширения/перехватчики). Если есть и исключение не THRequestInterceptionException, берём
его.
• TApplicationException.PROTOCOL_ERROR → TErrorType.PROTOCOL, причина «thrift protocol error».
• TApplicationException.UNKNOWN_METHOD → UNKNOWN_CALL, причина «unknown method: <имя>».
• Остальные TApplicationException → UNKNOWN, причина «unknown provider error: …».
• TProtocolException → PROTOCOL, причина «thrift protocol error».
• TTransportException → TRANSPORT, причина «thrift transport error».
• Если в метаданных статус 4xx → считаем, что это внешняя ошибка (generationSource = EXTERNAL, errorSource = EXTERNAL).
• Если статуса нет, но сообщение похоже на пустой ответ HTTP (HTTP response code: …, «No more data available.») → тоже
внешняя.
• THRequestInterceptionException (до сервиса не дошли) → TRANSPORT, подставляем подтип (BAD_HEADER, BAD_CONTENT_TYPE и т.
д.), генератором считаем:
• клиентский контекст → generationSource = INTERNAL (мы сами забраковали свой outbound запрос);
• серверный контекст → EXTERNAL (пришёл мусор извне).
• errorSource ставим равным generationSource, а причину формируем читаемо («bad header: X-Test», «content type
wrong/missing» и т. д.).
• Любое другое исключение → UNKNOWN, причина «internal thrift application error».
errorReason/errorMessage — по исключению.
обрабатывать.
──────────────────────────────────────────
• mapTransportErrorStatus переводит транспортные ошибки в коды: BAD_REQUEST_TYPE → 405, BAD_CONTENT_TYPE → 415, остальные →
400.
• isNoPayloadTransportError ловит текстовые сообщения TTransportException без полезной нагрузки («HTTP Response code: …»,
«No more data available.») и помечает их как внешние.
• Если перехватчик (ContextUtils.getInterceptionError) уже прервал запрос, getResponseInfo обязательно отдаёт 4xx,
независимо от дальнейших событий.
──────────────────────────────────────────
• Внешний шум (apстрим/сетевые ошибки) теперь показывается как generationSource=EXTERNAL, errorSource=EXTERNAL.
• Настоящие ошибки сервиса (бизнес, undefined/unavailable результат, внутренние 500) — generationSource=EXTERNAL, но
errorSource=INTERNAL, что отражает, что сбой произошёл у нас.
• Сообщения по 4xx/5xx нормализованы, чтобы клиент видел понятные Reason-Phrase.
• Тесты (THProviderErrorMapperTest, MDCLogTest, TestClientEventHandling, TestNginxError и пр.) покрывают эти комбинации и
подтверждают согласованность.
В итоге любой внешний потребитель по WErrorDefinition и HTTP-статусу точно понимает, кто виноват (мы или апстрим), и что за
проблема случилась.