-
Notifications
You must be signed in to change notification settings - Fork 2.4k
SSE Edge #4119
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
SSE Edge #4119
Changes from all commits
5589e79
51e1969
f4d3cb7
a8cb529
804d4cb
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,6 +1,7 @@ | ||
| import io from 'socket.io-client'; | ||
| import { dispatch } from 'codesandbox-api'; | ||
| import _debug from 'debug'; | ||
| import axios from 'axios'; | ||
|
|
||
| import { IExecutor, IFiles, ISetupParams } from './executor'; | ||
|
|
||
|
|
@@ -71,20 +72,18 @@ export class ServerExecutor implements IExecutor { | |
| this.token = this.retrieveSSEToken(); | ||
| } | ||
|
|
||
| private initializeSocket() { | ||
| private async initializeSocket() { | ||
| if (!this.sandboxId) { | ||
| throw new Error('initializeSocket: sandboxId is not defined'); | ||
| } | ||
|
|
||
| const usedHost = this.host || 'https://codesandbox.io'; | ||
| const sseHost = usedHost.replace('https://', 'https://sse.'); | ||
| const sseLbHost = usedHost.replace('https://', 'https://sse-lb.'); | ||
| const res = await axios.get(`${sseLbHost}/api/cluster/${this.sandboxId}`); | ||
| const sseHost = res.data.hostname; | ||
|
|
||
| this.socket = io(sseHost, { | ||
| autoConnect: false, | ||
| transports: ['websocket', 'polling'], | ||
| query: { | ||
| sandboxid: this.sandboxId, | ||
|
Member
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. Do we lose any data in the manager with this? Might there be a reason in the future that we want to infer the sandbox id from the WS connection? If that's the case we might want to keep it in the URL, but I'm not sure if you already have this data.
Contributor
Author
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. The client already sends the sandbox id to |
||
| }, | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -100,7 +99,7 @@ export class ServerExecutor implements IExecutor { | |
| await this.dispose(); | ||
| await tick(); | ||
|
|
||
| this.initializeSocket(); | ||
| await this.initializeSocket(); | ||
| } | ||
|
|
||
| public async setup() { | ||
|
|
@@ -110,8 +109,10 @@ export class ServerExecutor implements IExecutor { | |
| } | ||
|
|
||
| public async dispose() { | ||
| this.socket?.removeAllListeners(); | ||
| this.socket?.close(); | ||
| if (this.socket) { | ||
| this.socket.removeAllListeners(); | ||
| this.socket.close(); | ||
| } | ||
| } | ||
|
|
||
| public updateFiles(newFiles: IFiles) { | ||
|
|
@@ -131,11 +132,15 @@ export class ServerExecutor implements IExecutor { | |
| } | ||
|
|
||
| public emit(event: string, data?: any) { | ||
| this.socket?.emit(event, data); | ||
| if (this.socket) { | ||
| this.socket.emit(event, data); | ||
| } | ||
| } | ||
|
|
||
| public on(event: string, listener: (data: any) => void) { | ||
| this.socket?.on(event, listener); | ||
| if (this.socket) { | ||
| this.socket.on(event, listener); | ||
| } | ||
| } | ||
|
|
||
| private openSocket() { | ||
|
|
@@ -144,7 +149,7 @@ export class ServerExecutor implements IExecutor { | |
| } | ||
|
|
||
| return new Promise<void>((resolve, reject) => { | ||
| this.socket?.on('connect', async () => { | ||
| this.socket!.on('connect', async () => { | ||
| try { | ||
| if (this.connectTimeout) { | ||
| clearTimeout(this.connectTimeout); | ||
|
|
@@ -160,7 +165,7 @@ export class ServerExecutor implements IExecutor { | |
| } | ||
| }); | ||
|
|
||
| this.socket?.on('sandbox:start', () => { | ||
| this.socket!.on('sandbox:start', () => { | ||
| sseTerminalMessage(`Sandbox ${this.sandboxId} started`); | ||
| }); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -412,6 +412,13 @@ autoprefixer@^6.3.1: | |
| postcss "^5.2.16" | ||
| postcss-value-parser "^3.2.3" | ||
|
|
||
| axios@^0.19.2: | ||
| version "0.19.2" | ||
| resolved "https://registry.yarnpkg.com/axios/-/axios-0.19.2.tgz#3ea36c5d8818d0d5f8a8a97a6d36b86cdc00cb27" | ||
| integrity sha512-fjgm5MvRHLhx+osE2xoekY70AhARk3a6hkN+3Io1jc00jtquGvxYlKlsFUhmUET0V5te6CcZI7lcv2Ym61mjHA== | ||
| dependencies: | ||
| follow-redirects "1.5.10" | ||
|
|
||
| babel-code-frame@^6.26.0: | ||
| version "6.26.0" | ||
| resolved "https://registry.yarnpkg.com/babel-code-frame/-/babel-code-frame-6.26.0.tgz#63fd43f7dc1e3bb7ce35947db8fe369a3f58c74b" | ||
|
|
@@ -2005,7 +2012,7 @@ [email protected], debug@^2.1.2, debug@^2.2.0, debug@^2.3.3, debug@^2.6.8, debug@^2.6. | |
| dependencies: | ||
| ms "2.0.0" | ||
|
|
||
| debug@~3.1.0: | ||
| debug@=3.1.0, debug@~3.1.0: | ||
| version "3.1.0" | ||
| resolved "https://registry.yarnpkg.com/debug/-/debug-3.1.0.tgz#5bb5a0672628b64149566ba16819e61518c67261" | ||
| integrity sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g== | ||
|
|
@@ -2477,6 +2484,13 @@ flatten@^1.0.2: | |
| resolved "https://registry.yarnpkg.com/flatten/-/flatten-1.0.2.tgz#dae46a9d78fbe25292258cc1e780a41d95c03782" | ||
| integrity sha1-2uRqnXj74lKSJYzB54CkHZXAN4I= | ||
|
|
||
| [email protected]: | ||
| version "1.5.10" | ||
| resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.5.10.tgz#7b7a9f9aea2fdff36786a94ff643ed07f4ff5e2a" | ||
| integrity sha512-0V5l4Cizzvqt5D44aTXbFZz+FtyXV1vrDN6qrelxtfYQKW0KO0W2T/hkE8xvGa/540LkZlkaUjO4ailYTFtHVQ== | ||
| dependencies: | ||
| debug "=3.1.0" | ||
|
|
||
| fontfaceobserver@2.*: | ||
| version "2.0.13" | ||
| resolved "https://registry.yarnpkg.com/fontfaceobserver/-/fontfaceobserver-2.0.13.tgz#47adbb343261eda98cb44db2152196ff124d3221" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6643,7 +6643,7 @@ axe-core@^3.3.2: | |
| resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-3.4.1.tgz#e42623918bb85b5ef674633852cb9029db0309c5" | ||
| integrity sha512-+EhIdwR0hF6aeMx46gFDUy6qyCfsL0DmBrV3Z+LxYbsOd8e1zBaPHa3f9Rbjsz2dEwSBkLw6TwML/CAIIAqRpw== | ||
|
|
||
| [email protected], axios@^0.19.0: | ||
| [email protected]: | ||
| version "0.19.0" | ||
| resolved "https://registry.yarnpkg.com/axios/-/axios-0.19.0.tgz#8e09bff3d9122e133f7b8101c8fbdd00ed3d2ab8" | ||
| integrity sha512-1uvKqKQta3KBxIz14F2v06AEHZ/dIoeKfbTRkK1E5oqjDnuEerLmYTgJB5AiQZHJcljpg1TuRzdjDR06qNk0DQ== | ||
|
|
@@ -6659,6 +6659,13 @@ axios@^0.16.2: | |
| follow-redirects "^1.2.3" | ||
| is-buffer "^1.1.5" | ||
|
|
||
| axios@^0.19.0, axios@^0.19.2: | ||
| version "0.19.2" | ||
| resolved "https://registry.yarnpkg.com/axios/-/axios-0.19.2.tgz#3ea36c5d8818d0d5f8a8a97a6d36b86cdc00cb27" | ||
| integrity sha512-fjgm5MvRHLhx+osE2xoekY70AhARk3a6hkN+3Io1jc00jtquGvxYlKlsFUhmUET0V5te6CcZI7lcv2Ym61mjHA== | ||
| dependencies: | ||
| follow-redirects "1.5.10" | ||
|
|
||
| axobject-query@^2.0.2: | ||
| version "2.0.2" | ||
| resolved "https://registry.yarnpkg.com/axobject-query/-/axobject-query-2.0.2.tgz#ea187abe5b9002b377f925d8bf7d1c561adf38f9" | ||
|
|
||
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 we should put this in a function that retries up to 3 times, in case of flaky connections or when the LB has an error.
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.
That's a good point, do we use any retry library anywhere that we could reuse here? Also I think some error handling would be nice, in case all retries fail, but I'm not sure what the UI/UX pattern for this is.
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.
We don't use anything generic right now, I have written a poor mans retry function. Would be good to introduce some common helper function in
@codesandbox/commonor a library I think.Regarding the notice, we can send a notification from here. You can import
and do