Skip to content

[Enhancement] Expose auth/connectivity error details from connector metadata operations to end users#75490

Open
dirtysalt wants to merge 16 commits into
StarRocks:mainfrom
dirtysalt:expose-auth-error
Open

[Enhancement] Expose auth/connectivity error details from connector metadata operations to end users#75490
dirtysalt wants to merge 16 commits into
StarRocks:mainfrom
dirtysalt:expose-auth-error

Conversation

@dirtysalt

@dirtysalt dirtysalt commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Why I'm doing:

When external catalogs (HMS, Glue, Iceberg REST/Glue, Delta Lake, Hudi, Paimon) hit an auth or connectivity failure, the error shown to users was often incomplete — a single-level message like "The security token is invalid" with no indication of which layer threw it, since each connector handled exceptions inconsistently and the display site only ever called e.getMessage(), which surfaces just the outermost message.

What I'm doing:

Lower layers stop flattening or re-interpreting the exception chain — they only add a brief context label. The top-level display site unwinds the full chain once and presents it to the user.

connector layer:  throw new StarRocksConnectorException("Failed to list tables for catalog X", cause)
                  └─ only adds context; cause chain preserved natively via getCause()

display site:     LogUtil.getUnwoundExceptionMessage(e)
                  └─ walks the getCause() chain, keeps the outer message plain,
                     and prefixes deeper layers with their class name
                  e.g.:  Failed to list tables for catalog X
                         GlueException: The security token is invalid (Service: Glue, Status Code: 400)
  • LogUtil.getUnwoundExceptionMessage(Throwable) — walks the cause chain up to 20 levels, transparently unwraps InvocationTargetException (common from HMS reflection calls), and formats the result.
  • StmtExecutor — both display paths (catch StarRocksException for DDL and catch Throwable for queries/SHOW) now call getUnwoundExceptionMessage, so the full chain reaches the user regardless of which connector or operation triggered the error.
  • Connector call sites — replaced inconsistent patterns with uniform new StarRocksConnectorException(context, cause) across HiveMetaClient.callRPC(), IcebergRESTCatalog, IcebergGlueCatalog, CachingIcebergCatalog, IcebergMetadata, DeltaLakeMetastore/DeltaLakeMetadata, HudiMetadata, PaimonMetadata.
  • StarRocksConnectorException — removed the dead fromExternalException-era overrides.

Scoping fix (added after initial review): StmtExecutor's two catch blocks are the shared error-reporting path for every SQL statement, not just connector operations, so a naive "always unwind" implementation leaked class-name prefixes into unrelated errors (e.g. a plain DdlException: Unknown thread id: ...) that never had them before. getUnwoundExceptionMessage now only produces the multi-layer format when there's a genuine, distinct cause chain to unwind:

  • A single-layer exception (no real cause) falls back to the exact original e.getMessage() — no class name.
  • Adjacent layers carrying an identical message (a pattern already used internally to re-wrap exceptions for classification, e.g. SemanticExceptionAnalysisException) are collapsed into one so the same text isn't shown twice.
  • For genuine multi-layer chains, the outermost layer is still shown plain (it's almost always a generic internal wrapper type) — only deeper layers get a class-name prefix, since that's where the "which system actually failed" signal lives.

This keeps the original connector-error goal intact while leaving every unrelated error message byte-for-byte unchanged.

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.1
    • 4.0
    • 3.5

@CelerData-Reviewer

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: b6f18bf1df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@CelerData-Reviewer

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5e3d3cf92

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fe/fe-core/src/test/java/com/starrocks/common/util/LogUtilTest.java Outdated
@dirtysalt dirtysalt enabled auto-merge (squash) June 28, 2026 11:18
Signed-off-by: yanz <dirtysalt1987@gmail.com>

Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
@dirtysalt dirtysalt force-pushed the expose-auth-error branch from 4d5e063 to 743a4bb Compare June 30, 2026 02:31
@CelerData-Reviewer

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 743a4bba1f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fe/fe-core/src/main/java/com/starrocks/qe/DDLStmtExecutor.java Outdated
@dirtysalt dirtysalt marked this pull request as draft June 30, 2026 03:14
auto-merge was automatically disabled June 30, 2026 03:14

Pull request was converted to draft

Signed-off-by: tqqq <dirtysalt1987@gmail.com>
@dirtysalt dirtysalt marked this pull request as ready for review June 30, 2026 03:58
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
@CelerData-Reviewer

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5dcd2fe2fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
@dirtysalt dirtysalt closed this Jun 30, 2026
@dirtysalt

dirtysalt commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

I guess probabbly no one is interested in that since this enhancement needs broad and long time discussion

@dirtysalt dirtysalt deleted the expose-auth-error branch July 1, 2026 04:12
@dirtysalt dirtysalt restored the expose-auth-error branch July 14, 2026 08:26
@dirtysalt dirtysalt reopened this Jul 14, 2026
@dirtysalt dirtysalt enabled auto-merge (squash) July 14, 2026 08:27
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
@CelerData-Reviewer

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c62e17917

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: tqqq <dirtysalt1987@gmail.com>
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
@CelerData-Reviewer

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 49fcd485e0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

@github-actions

Copy link
Copy Markdown
Contributor

[FE Incremental Coverage Report]

pass : 81 / 90 (90.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/connector/iceberg/IcebergMetadata.java 0 5 00.00% [466, 467, 711, 712, 1201]
🔵 com/starrocks/connector/hive/HiveMetaClient.java 1 2 50.00% [461]
🔵 com/starrocks/connector/paimon/PaimonMetadata.java 2 4 50.00% [185, 186]
🔵 com/starrocks/connector/iceberg/rest/IcebergRESTCatalog.java 15 16 93.75% [130]
🔵 com/starrocks/common/util/LogUtil.java 41 41 100.00% []
🔵 com/starrocks/connector/iceberg/CachingIcebergCatalog.java 2 2 100.00% []
🔵 com/starrocks/qe/StmtExecutor.java 2 2 100.00% []
🔵 com/starrocks/connector/hudi/HudiMetadata.java 2 2 100.00% []
🔵 com/starrocks/connector/delta/CachingDeltaLakeMetastore.java 1 1 100.00% []
🔵 com/starrocks/connector/delta/DeltaLakeMetastore.java 6 6 100.00% []
🔵 com/starrocks/connector/delta/DeltaLakeMetadata.java 2 2 100.00% []
🔵 com/starrocks/connector/iceberg/glue/IcebergGlueCatalog.java 5 5 100.00% []
🔵 com/starrocks/connector/hive/CachingHiveMetastore.java 2 2 100.00% []

@sonarqubecloud

Copy link
Copy Markdown

@CelerData-Reviewer

Copy link
Copy Markdown

@codex review

@github-actions

Copy link
Copy Markdown
Contributor

[BE Incremental Coverage Report]

pass : 0 / 0 (0%)

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: a8b414fb32

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants