-
Notifications
You must be signed in to change notification settings - Fork 96
Add optional timeouts to clients in server APIs #560
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
Changes from 2 commits
ac3ef5a
769f01d
08c8cda
55890cb
84ad947
59bdca9
2449cda
e6b06fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,10 @@ | |
| metadata?: string; | ||
| } | ||
|
|
||
| type ClientOptions = { | ||
| requestTimeout?: number; | ||
| }; | ||
|
|
||
| const svc = 'AgentDispatchService'; | ||
|
|
||
| /** | ||
|
|
@@ -29,10 +33,15 @@ | |
| * @param host - hostname including protocol. i.e. 'https://<project>.livekit.cloud' | ||
| * @param apiKey - API Key, can be set in env var LIVEKIT_API_KEY | ||
| * @param secret - API Secret, can be set in env var LIVEKIT_API_SECRET | ||
| * @param options - client options | ||
| * @param options.requestTimeout - optional timeout, in seconds, for all server requests | ||
|
Check warning on line 37 in packages/livekit-server-sdk/src/AgentDispatchClient.ts
|
||
| */ | ||
| constructor(host: string, apiKey?: string, secret?: string) { | ||
| constructor(host: string, apiKey?: string, secret?: string, options?: ClientOptions) { | ||
| super(apiKey, secret); | ||
| this.rpc = new TwirpRpc(host, livekitPackage); | ||
| const rpcOptions = options?.requestTimeout | ||
| ? { requestTimeout: options.requestTimeout } | ||
| : undefined; | ||
| this.rpc = new TwirpRpc(host, livekitPackage, rpcOptions); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,6 +127,10 @@ | |
| active?: boolean; | ||
| } | ||
|
|
||
| type ClientOptions = { | ||
| requestTimeout?: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Client to access Egress APIs | ||
| */ | ||
|
|
@@ -137,10 +141,15 @@ | |
| * @param host - hostname including protocol. i.e. 'https://<project>.livekit.cloud' | ||
| * @param apiKey - API Key, can be set in env var LIVEKIT_API_KEY | ||
| * @param secret - API Secret, can be set in env var LIVEKIT_API_SECRET | ||
| * @param options - client options | ||
| * @param options.requestTimeout - optional timeout, in seconds, for all server requests | ||
|
Check warning on line 145 in packages/livekit-server-sdk/src/EgressClient.ts
|
||
| */ | ||
| constructor(host: string, apiKey?: string, secret?: string) { | ||
| constructor(host: string, apiKey?: string, secret?: string, options?: ClientOptions) { | ||
| super(apiKey, secret); | ||
| this.rpc = new TwirpRpc(host, livekitPackage); | ||
| const rpcOptions = options?.requestTimeout | ||
| ? { requestTimeout: options.requestTimeout } | ||
| : undefined; | ||
| this.rpc = new TwirpRpc(host, livekitPackage, rpcOptions); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,10 @@ | |
| ingressId?: string; | ||
| } | ||
|
|
||
| type ClientOptions = { | ||
| requestTimeout?: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Client to access Ingress APIs | ||
| */ | ||
|
|
@@ -124,10 +128,15 @@ | |
| * @param host - hostname including protocol. i.e. 'https://<project>.livekit.cloud' | ||
| * @param apiKey - API Key, can be set in env var LIVEKIT_API_KEY | ||
| * @param secret - API Secret, can be set in env var LIVEKIT_API_SECRET | ||
| * @param options - client options | ||
| * @param options.requestTimeout - optional timeout, in seconds, for all server requests | ||
|
Check warning on line 132 in packages/livekit-server-sdk/src/IngressClient.ts
|
||
| */ | ||
| constructor(host: string, apiKey?: string, secret?: string) { | ||
| constructor(host: string, apiKey?: string, secret?: string, options?: ClientOptions) { | ||
| super(apiKey, secret); | ||
| this.rpc = new TwirpRpc(host, livekitPackage); | ||
| const rpcOptions = options?.requestTimeout | ||
| ? { requestTimeout: options.requestTimeout } | ||
| : undefined; | ||
| this.rpc = new TwirpRpc(host, livekitPackage, rpcOptions); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,6 +107,10 @@ | |
| name?: string; | ||
| }; | ||
|
|
||
| type ClientOptions = { | ||
| requestTimeout?: number; | ||
| }; | ||
|
|
||
| const svc = 'RoomService'; | ||
|
|
||
| /** | ||
|
|
@@ -120,10 +124,15 @@ | |
| * @param host - hostname including protocol. i.e. 'https://<project>.livekit.cloud' | ||
| * @param apiKey - API Key, can be set in env var LIVEKIT_API_KEY | ||
| * @param secret - API Secret, can be set in env var LIVEKIT_API_SECRET | ||
| * @param options - client options | ||
| * @param options.requestTimeout - optional timeout, in seconds, for all server requests | ||
|
Check warning on line 128 in packages/livekit-server-sdk/src/RoomServiceClient.ts
|
||
| */ | ||
| constructor(host: string, apiKey?: string, secret?: string) { | ||
| constructor(host: string, apiKey?: string, secret?: string, options?: ClientOptions) { | ||
| super(apiKey, secret); | ||
| this.rpc = new TwirpRpc(host, livekitPackage); | ||
| const rpcOptions = options?.requestTimeout | ||
| ? { requestTimeout: options.requestTimeout } | ||
| : undefined; | ||
| this.rpc = new TwirpRpc(host, livekitPackage, rpcOptions); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -205,6 +205,10 @@ | |
| headers?: { [key: string]: string }; | ||
| } | ||
|
|
||
| type ClientOptions = { | ||
| requestTimeout?: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Client to access Egress APIs | ||
| */ | ||
|
|
@@ -215,10 +219,15 @@ | |
| * @param host - hostname including protocol. i.e. 'https://<project>.livekit.cloud' | ||
| * @param apiKey - API Key, can be set in env var LIVEKIT_API_KEY | ||
| * @param secret - API Secret, can be set in env var LIVEKIT_API_SECRET | ||
| * @param options - client options | ||
| * @param options.requestTimeout - optional timeout, in seconds, for all server requests | ||
|
Check warning on line 223 in packages/livekit-server-sdk/src/SipClient.ts
|
||
| */ | ||
| constructor(host: string, apiKey?: string, secret?: string) { | ||
| constructor(host: string, apiKey?: string, secret?: string, options?: ClientOptions) { | ||
| super(apiKey, secret); | ||
| this.rpc = new TwirpRpc(host, livekitPackage); | ||
| const rpcOptions = options?.requestTimeout | ||
| ? { requestTimeout: options.requestTimeout } | ||
| : undefined; | ||
| this.rpc = new TwirpRpc(host, livekitPackage, rpcOptions); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,15 @@ import type { JsonValue } from '@bufbuild/protobuf'; | |
|
|
||
| // twirp RPC adapter for client implementation | ||
|
|
||
| type Options = { | ||
| /** Prefix for the RPC requests */ | ||
| prefix?: string; | ||
| /** Timeout for fetch requests, in seconds. Must resolve to a positive integer millisecond value. */ | ||
|
||
| requestTimeout?: number; | ||
| }; | ||
|
|
||
| const defaultPrefix = '/twirp'; | ||
| const defaultTimeoutSeconds = 60; | ||
|
|
||
| export const livekitPackage = 'livekit'; | ||
| export interface Rpc { | ||
|
|
@@ -48,21 +56,24 @@ export class TwirpRpc { | |
|
|
||
| prefix: string; | ||
|
|
||
| constructor(host: string, pkg: string, prefix?: string) { | ||
| requestTimeout: number; | ||
|
|
||
| constructor(host: string, pkg: string, options?: Options) { | ||
| if (host.startsWith('ws')) { | ||
| host = host.replace('ws', 'http'); | ||
| } | ||
| this.host = host; | ||
| this.pkg = pkg; | ||
| this.prefix = prefix || defaultPrefix; | ||
| this.requestTimeout = options?.requestTimeout ?? defaultTimeoutSeconds; | ||
| this.prefix = options?.prefix || defaultPrefix; | ||
| } | ||
|
|
||
| async request( | ||
| service: string, | ||
| method: string, | ||
| data: any, // eslint-disable-line @typescript-eslint/no-explicit-any | ||
| headers: any, // eslint-disable-line @typescript-eslint/no-explicit-any | ||
| timeout = 60, | ||
| timeout = this.requestTimeout, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| ): Promise<any> { | ||
| const path = `${this.prefix}/${this.pkg}.${service}/${method}`; | ||
|
|
||
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.
Instead of describing the usage here, could you do it just above the field definition in the
ClientOptionstype?Would also be nice if we could share the
ClientOptionstype across different implementations instead of redefining it in each client module.