feat: add @SdkAdvancedApi annotation for error-prone APIs - #7203
Open
zoewangg wants to merge 4 commits into
Open
feat: add @SdkAdvancedApi annotation for error-prone APIs#7203zoewangg wants to merge 4 commits into
zoewangg wants to merge 4 commits into
Conversation
Introduce @SdkAdvancedApi in core/annotations to mark APIs that are error-prone to implement, override, call, or configure: misuse compiles cleanly but can fail or misbehave at runtime rather than reporting a clear error. The annotation carries structured, machine-readable guidance (caution kind, contract explanation, safer alternative, and a documentation link) with CLASS retention so build-time or static-analysis tooling can consume it, matching the existing @NotThreadSafe/@immutable markers. Apply it to the streaming and interceptor extension points identified as error-prone: AsyncRequestBody (type + fromPublisher), AsyncResponseTransformer (type + toPublisher overloads), ResponseTransformer, ContentStreamProvider, the mutating ExecutionInterceptor content hooks (modifyHttpContent, modifyAsyncHttpContent), and the FUTURE_COMPLETION_EXECUTOR advanced client option. This is an advisory marker only: it does not gate compilation and adds no runtime behavior, so it is source and binary compatible.
Add a source-AST checkstyle check that fails the build when a guidance or saferAlternative value on @SdkAdvancedApi exceeds 1000 characters. Values are multi-line string concatenations, so the check sums the decoded length of every concatenated literal. Keeps the text short enough to be useful inline; the longest current message is 637.
zoewangg
force-pushed
the
zoewang/advanced-api-annotation
branch
from
July 29, 2026 19:56
106faf2 to
2942141
Compare
dagnir
reviewed
Jul 29, 2026
Fred1155
reviewed
Jul 29, 2026
| @SdkPublicApi | ||
| @SdkAdvancedApi( | ||
| caution = Caution.WHEN_IMPLEMENTED, | ||
| guidance = "transform() must fully drain and close the response stream and honor thread interrupts. A " |
Contributor
There was a problem hiding this comment.
This seems contradict with Javadoc, and is the "must close" wording accurate? I think it does not need to close the response stream since CombinedResponseHandler will close it?
Contributor
Author
There was a problem hiding this comment.
Good catch! I don't think we actually need advanced API annotation for this API, will remove
Correct the ResponseTransformer guidance: the SDK (via CombinedResponseHandler) drains and closes the response stream after transform() returns, so the implementation does not need to close it; the prior 'must close' wording contradicted the interface Javadoc. Reword the FUTURE_COMPLETION_EXECUTOR guidance to describe the risky executor as one that does not dispatch to a separate thread rather than naming an I/O thread. Minor formatting reflow in the AsyncResponseTransformer guidance string.
Scope the initial rollout to the higher-severity extension points. The sync ResponseTransformer is a plain callback whose only contract is thread-interrupt handling (low priority); revisit if warranted.
Fred1155
approved these changes
Jul 30, 2026
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
3 participants
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.
Motivation and Context
The SDK exposes several extension points that customers implement, override, call, or configure to customize streaming and request/response processing. Each carries a strict, often subtle contract, and a non-compliant use compiles cleanly but fails or misbehaves at runtime rather than reporting a clear error. Today that risk is described only in prose Javadoc, written differently per API and in a form no tool can read. This change adds a single, uniform, machine-readable marker for that class of error-prone API, which is the prerequisite for future build-time or static-analysis/AI tooling that can warn on misuse.
Modifications
@SdkAdvancedApiincore/annotations(CLASS retention,@Documented, targets TYPE/METHOD/CONSTRUCTOR/FIELD). Members:caution(required; WHEN_IMPLEMENTED / WHEN_OVERRIDDEN / WHEN_CONFIGURED / WHEN_CALLED),guidance(required),saferAlternative(optional),link(optional).AsyncRequestBody(type +fromPublisher),AsyncResponseTransformer(type +toPublisher()/toPublisher(Duration)),ResponseTransformer,ContentStreamProvider, the mutatingExecutionInterceptorhooks (modifyHttpContent,modifyAsyncHttpContent), and theFUTURE_COMPLETION_EXECUTORadvanced client option.guidance/saferAlternativelength at 1000 characters.Testing
mvn installoncore/annotations,http-client-spi,core/sdk-core, andbuild-tools: BUILD SUCCESS.Types of changes
Checklist
mvn installsucceedsLicense