-
Notifications
You must be signed in to change notification settings - Fork 308
feat(pyth-lazer-stk): correctly handle connection drops + other improvements #2737
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 all commits
dac3ec1
e01df80
4d89633
61232af
37b1b4b
ee81ea5
85fec78
8a9a17a
d8dc385
c7b6241
30336d1
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 |
|---|---|---|
| @@ -1,9 +1,8 @@ | ||
| import WebSocket from "isomorphic-ws"; | ||
| import type { Logger } from "ts-log"; | ||
| import { dummyLogger } from "ts-log"; | ||
|
|
||
| import type { ParsedPayload, Request, Response } from "./protocol.js"; | ||
| import { BINARY_UPDATE_FORMAT_MAGIC_LE, FORMAT_MAGICS_LE } from "./protocol.js"; | ||
| import type { WebSocketPoolConfig } from "./socket/websocket-pool.js"; | ||
| import { WebSocketPool } from "./socket/websocket-pool.js"; | ||
|
|
||
| export type BinaryResponse = { | ||
|
|
@@ -35,13 +34,8 @@ export class PythLazerClient { | |
| * @param numConnections - The number of parallel WebSocket connections to establish (default: 3). A higher number gives a more reliable stream. The connections will round-robin across the provided URLs. | ||
| * @param logger - Optional logger to get socket level logs. Compatible with most loggers such as the built-in console and `bunyan`. | ||
| */ | ||
| static async create( | ||
| urls: string[], | ||
| token: string, | ||
| numConnections = 3, | ||
| logger: Logger = dummyLogger, | ||
| ): Promise<PythLazerClient> { | ||
| const wsp = await WebSocketPool.create(urls, token, numConnections, logger); | ||
| static async create(config: WebSocketPoolConfig): Promise<PythLazerClient> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit of a nit, and not something you added here, but for future reference IMO static methods are an antipattern. Unless you have a good reason why you really need the
|
||
| const wsp = await WebSocketPool.create(config); | ||
| return new PythLazerClient(wsp); | ||
| } | ||
|
|
||
|
|
@@ -102,19 +96,19 @@ export class PythLazerClient { | |
| }); | ||
| } | ||
|
|
||
| async subscribe(request: Request): Promise<void> { | ||
| subscribe(request: Request) { | ||
| if (request.type !== "subscribe") { | ||
| throw new Error("Request must be a subscribe request"); | ||
| } | ||
| await this.wsp.addSubscription(request); | ||
| this.wsp.addSubscription(request); | ||
| } | ||
|
|
||
| async unsubscribe(subscriptionId: number): Promise<void> { | ||
| await this.wsp.removeSubscription(subscriptionId); | ||
| unsubscribe(subscriptionId: number) { | ||
| this.wsp.removeSubscription(subscriptionId); | ||
| } | ||
|
|
||
| async send(request: Request): Promise<void> { | ||
| await this.wsp.sendRequest(request); | ||
| send(request: Request) { | ||
| this.wsp.sendRequest(request); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
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.
I remember one of the early consumers of the SDK wanted the ability to
awaitthesubscribe()(i.e. promise is fulfilled when the lazer server acks the subscription.) Is that no longer a requirement? Personally I think it makes for nicer ergonomics to keep it async.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.
oh yeah good call. I'll check it.