-
Couldn't load subscription status.
- Fork 5.5k
feat(sdk-package): call apis directly from client sdk #14699
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
a8e6b7f
065cefa
87e47af
6343663
2f4e8e6
b8bba0a
41d44cf
7ba8e48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { AsyncResponseManager } from "../shared/async"; | ||
| import type { AsyncResponseManagerOpts } from "../shared/async"; | ||
|
|
||
| export type BrowserAsyncResponseManagerOpts = { | ||
| apiHost: string; | ||
| getConnectToken: () => Promise<string>; | ||
| }; | ||
|
|
||
| export class BrowserAsyncResponseManager extends AsyncResponseManager { | ||
| private browserOpts: BrowserAsyncResponseManagerOpts; | ||
|
|
||
| constructor(opts: BrowserAsyncResponseManagerOpts) { | ||
| super(); | ||
| this.browserOpts = opts; | ||
| } | ||
|
|
||
| protected override async getOpts(): Promise<AsyncResponseManagerOpts> { | ||
| const token = await this.browserOpts.getConnectToken(); | ||
| const url = `wss://${this.browserOpts.apiHost}/websocket?ctok=${token}`; | ||
| return { | ||
| url, | ||
| }; | ||
| } | ||
|
Comment on lines
+17
to
+23
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. Add error handling and security enhancements The WebSocket URL construction and token handling could benefit from additional security measures and error handling:
Here's a suggested implementation: protected override async getOpts(): Promise<AsyncResponseManagerOpts> {
- const token = await this.browserOpts.getConnectToken();
- const url = `wss://${this.browserOpts.apiHost}/websocket?ctok=${token}`;
+ let token: string;
+ try {
+ token = await this.browserOpts.getConnectToken();
+ if (!token?.trim()) {
+ throw new Error('Invalid token received');
+ }
+ } catch (error) {
+ throw new Error(`Failed to retrieve connect token: ${error.message}`);
+ }
+
+ const url = new URL(`wss://${this.browserOpts.apiHost}/websocket`);
return {
url,
+ headers: {
+ 'Authorization': `Bearer ${token}`
+ }
};
}
|
||
| } | ||
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.
🛠️ Refactor suggestion
Enhance the deprecation notice with migration guidance
The deprecation notice for the
environmentsproperty should include:Consider updating the changelog entry like this:
📝 Committable suggestion