Skip to content

feat: add @SdkAdvancedApi annotation for error-prone APIs - #7203

Merged
zoewangg merged 5 commits into
masterfrom
zoewang/advanced-api-annotation
Jul 31, 2026
Merged

feat: add @SdkAdvancedApi annotation for error-prone APIs#7203
zoewangg merged 5 commits into
masterfrom
zoewang/advanced-api-annotation

Conversation

@zoewangg

@zoewangg zoewangg commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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

  • Add @SdkAdvancedApi in core/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).
  • Apply it to the error-prone extension points: AsyncRequestBody (type + fromPublisher), AsyncResponseTransformer (type + toPublisher() / toPublisher(Duration)), ResponseTransformer, ContentStreamProvider, the mutating ExecutionInterceptor hooks (modifyHttpContent, modifyAsyncHttpContent), and the FUTURE_COMPLETION_EXECUTOR advanced client option.
  • Add a checkstyle check capping guidance/saferAlternative length at 1000 characters.
  • The annotation is advisory only: it does not gate compilation and adds no runtime behavior.

Testing

  • mvn install on core/annotations, http-client-spi, core/sdk-core, and build-tools: BUILD SUCCESS.
  • japicmp reports no binary/source-incompatible modifications in any affected module (adding a CLASS-retention marker is compatible).
  • New checkstyle check has 5 unit tests (pass/fail/concatenation/boundary cases).

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added a changelog entry (scripts/new-change)

License

  • I confirm that this pull request can be released under the Apache 2 license

@zoewangg
zoewangg requested a review from a team as a code owner July 29, 2026 18:04
zoewangg added 2 commits July 29, 2026 12:47
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
zoewangg force-pushed the zoewang/advanced-api-annotation branch from 106faf2 to 2942141 Compare July 29, 2026 19:56
@zoewangg
zoewangg requested review from Fred1155 and dagnir July 29, 2026 20:08
zoewangg added 2 commits July 29, 2026 15:06
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.
Per API review feedback: cautionWhen = Usage.IMPLEMENTED reads more naturally than caution = Caution.WHEN_IMPLEMENTED. Values drop the WHEN_ prefix since the member now carries it.
@zoewangg zoewangg added the api-surface-area-approved-by-team Indicate API surface area introduced by this PR has been approved by team label Jul 30, 2026
@zoewangg
zoewangg added this pull request to the merge queue Jul 30, 2026
Merged via the queue into master with commit 595ab6f Jul 31, 2026
15 of 17 checks passed
@github-actions

Copy link
Copy Markdown

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api-surface-area-approved-by-team Indicate API surface area introduced by this PR has been approved by team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants