Conversation
🦋 Changeset detectedLatest commit: 3a63e57 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
496add1 to
f9950ce
Compare
Contributor
Changeset File Check ✅
|
Collaborator
Size Report 1Affected Products
Test Logs |
Collaborator
Size Analysis Report 1Affected Products
Test Logs |
b9a363a to
3e0cc6b
Compare
b3e7dd8 to
c616128
Compare
DellaBitta
reviewed
May 12, 2025
Contributor
DellaBitta
left a comment
There was a problem hiding this comment.
Looks good, but won't approve until council review.
|
|
||
| If provided, calling `abort()` on the corresponding `AbortController` will attempt to cancel the underlying HTTP request. An `AbortError` will be thrown if cancellation is successful. | ||
|
|
||
| Note that this will not cancel the request in the backend, so billing will still be applied despite cancellation. |
DellaBitta
approved these changes
May 12, 2025
Contributor
Author
|
Re-requesting review since changes were made. |
hsubox76
reviewed
Nov 18, 2025
Contributor
hsubox76
left a comment
There was a problem hiding this comment.
Weren't we going to have the iterator throw too if there was an error on that iteration? Does this do that? Did it already do that?
Contributor
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for AbortSignal across various methods, allowing for cancellation of in-flight requests. The implementation is robust, using AbortSignal.any() to gracefully handle both user-initiated aborts and internal timeouts. The changes are well-tested with new integration and unit tests. I've left a few comments on potential improvements, mainly around ensuring atomicity of history updates in ChatSession to prevent inconsistent states, and a couple of minor cleanup items in generated documentation files. Overall, this is a great addition to the SDK.
common/api-review/ai.api.md
Outdated
Comment on lines
+1341
to
+1342
| generateContent(templateId: string, templateVariables: object, // anything! | ||
| singleRequestOptions?: SingleRequestOptions): Promise<GenerateContentResult>; |
Contributor
There was a problem hiding this comment.
There appears to be a leftover comment // anything! in the generateContent method signature. This should be removed for clarity in the public API documentation.
Suggested change
| generateContent(templateId: string, templateVariables: object, // anything! | |
| singleRequestOptions?: SingleRequestOptions): Promise<GenerateContentResult>; | |
| generateContent(templateId: string, templateVariables: object, singleRequestOptions?: SingleRequestOptions): Promise<GenerateContentResult>; |
Comment on lines
+88
to
+89
| generateContent(templateId: string, templateVariables: object, // anything! | ||
| singleRequestOptions?: SingleRequestOptions): Promise<GenerateContentResult>; |
Contributor
There was a problem hiding this comment.
Similar to the API report file, there's a leftover comment // anything! in the generateContent method signature in this documentation file. It should be removed.
Suggested change
| generateContent(templateId: string, templateVariables: object, // anything! | |
| singleRequestOptions?: SingleRequestOptions): Promise<GenerateContentResult>; | |
| generateContent(templateId: string, templateVariables: object, singleRequestOptions?: SingleRequestOptions): Promise<GenerateContentResult>; |
hsubox76
approved these changes
Dec 1, 2025
Contributor
hsubox76
left a comment
There was a problem hiding this comment.
LG, thanks for adding the comments and the tests.
Merged
maneesht
pushed a commit
that referenced
this pull request
Feb 6, 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
5 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.
Add an
AbortSignalto a newSingleRequestOptionsthat can be passed to all methods that make requests to the backend, allowing them to be aborted.Fixes #8859
API Proposal: go/vinf-abort-request-api (internal)