-
Notifications
You must be signed in to change notification settings - Fork 347
Add custom abort controller for requests #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be very much appreciated, but IMO the public interfaces should use AbortSignal
instead.
console.log('\nAborting request...\n') | ||
ollama.abort() | ||
}, 1000) // 1000 milliseconds = 1 second | ||
}, 500) // 1000 milliseconds = 1 second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not needed
|
||
export interface GenerateRequest { | ||
interface AbortableRequest { | ||
abortController?: AbortController |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what I've seen, consummers are generally taking an AbortSignal
named signal
, and not an AbortController
. The user can provide it through new AbortController().signal
, AbortSignal.timeout()
or any other producer. See for example https://developer.mozilla.org/en-US/docs/Web/API/RequestInit#signal
const itr = parseJSON<T | ErrorResponse>(response.body) | ||
const abortableAsyncIterator = new AbortableAsyncIterator( | ||
abortController, | ||
request.abortController ?? abortController, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If taking an AbortSignal
as suggested in src/interfaces.ts
, you could instead call abortController.abort()
when the signal emits to propagate.
Abort signal was working only for ReadableStream after a request was sent. Now, it works even at the beginning of the request.
This could be integrated better but I didn't want to refactor the AbortableAsyncIterator structure.