Skip to content

Spawn vscode on demand #4499

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

Merged
merged 6 commits into from
Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions src/node/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { HttpCode, HttpError } from "../../common/http"
import { plural } from "../../common/util"
import { App } from "../app"
import { AuthType, DefaultedArgs } from "../cli"
import { commit, isDevMode, rootPath } from "../constants"
import { commit, rootPath } from "../constants"
import { Heart } from "../heart"
import { ensureAuthenticated, redirect } from "../http"
import { PluginAPI } from "../plugin"
Expand All @@ -23,7 +23,7 @@ import * as login from "./login"
import * as logout from "./logout"
import * as pathProxy from "./pathProxy"
import * as update from "./update"
import { createVSServerRouter, VSServerResult } from "./vscode"
import { CodeServerRouteWrapper } from "./vscode"

/**
* Register all routes and middleware.
Expand Down Expand Up @@ -138,20 +138,12 @@ export const register = async (app: App, args: DefaultedArgs): Promise<Disposabl

app.router.use("/update", update.router)

let vscode: VSServerResult
try {
vscode = await createVSServerRouter(args)
app.router.use("/", vscode.router)
app.wsRouter.use("/", vscode.wsRouter.router)
app.router.use("/vscode", vscode.router)
app.wsRouter.use("/vscode", vscode.wsRouter.router)
} catch (error: any) {
if (isDevMode) {
logger.warn(error)
logger.warn("VS Server router may still be compiling.")
} else {
throw error
}
const vsServerRouteHandler = new CodeServerRouteWrapper()

// Note that the root route is replaced in Coder Enterprise by the plugin API.
for (const routePrefix of ["/", "/vscode"]) {
app.router.use(routePrefix, vsServerRouteHandler.router)
app.wsRouter.use(routePrefix, vsServerRouteHandler.wsRouter)
Comment on lines +144 to +146
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so much cleaner 👏

}

app.router.use(() => {
Expand All @@ -164,6 +156,6 @@ export const register = async (app: App, args: DefaultedArgs): Promise<Disposabl
return () => {
heart.dispose()
pluginApi?.dispose()
vscode?.codeServerMain.dispose()
vsServerRouteHandler.dispose()
}
}
112 changes: 76 additions & 36 deletions src/node/routes/vscode.ts
Original file line number Diff line number Diff line change
@@ -1,65 +1,105 @@
import { logger } from "@coder/logger"
import * as express from "express"
import { DefaultedArgs } from "../cli"
import { WebsocketRequest } from "../../../typings/pluginapi"
import { logError } from "../../common/util"
import { isDevMode } from "../constants"
import { ensureAuthenticated, authenticated, redirect } from "../http"
import { loadAMDModule } from "../util"
import { Router as WsRouter, WebsocketRouter } from "../wsRouter"
import { Router as WsRouter } from "../wsRouter"
import { errorHandler } from "./errors"

export interface VSServerResult {
router: express.Router
wsRouter: WebsocketRouter
codeServerMain: CodeServerLib.IServerAPI
}

export const createVSServerRouter = async (args: DefaultedArgs): Promise<VSServerResult> => {
// See ../../../vendor/modules/code-oss-dev/src/vs/server/main.js.
const createVSServer = await loadAMDModule<CodeServerLib.CreateServer>(
"vs/server/remoteExtensionHostAgent",
"createServer",
)
export class CodeServerRouteWrapper {
/** Assigned in `ensureCodeServerLoaded` */
private _codeServerMain!: CodeServerLib.IServerAPI
private _wsRouterWrapper = WsRouter()
public router = express.Router()

const codeServerMain = await createVSServer(null, {
connectionToken: "0000",
...args,
// For some reason VS Code takes the port as a string.
port: typeof args.port !== "undefined" ? args.port.toString() : undefined,
})
public get wsRouter() {
return this._wsRouterWrapper.router
}

const router = express.Router()
const wsRouter = WsRouter()
//#region Route Handlers

router.get("/", async (req, res, next) => {
private $root: express.Handler = async (req, res, next) => {
const isAuthenticated = await authenticated(req)

if (!isAuthenticated) {
return redirect(req, res, "login", {
// req.baseUrl can be blank if already at the root.
to: req.baseUrl && req.baseUrl !== "/" ? req.baseUrl : undefined,
})
}

next()
})
}

router.all("*", ensureAuthenticated, (req, res, next) => {
req.on("error", (error: any) => {
private $proxyRequest: express.Handler = async (req, res, next) => {
// We allow certain errors to propagate so that other routers may handle requests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

❓ why start the name with $? is that some sort of special pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s something of a convention I’ve seen help with readability, similarly to prefixing private methods with _. Prefixing a method like $proxyRequest helps indicate a unique purpose compared to a more lengthy proxyRequestHandler

// outside VS Code
const requestErrorHandler = (error: any) => {
if (error instanceof Error && ["EntryNotFound", "FileNotFound", "HttpError"].includes(error.message)) {
next()
}

errorHandler(error, req, res, next)
})
}

req.once("error", requestErrorHandler)

codeServerMain.handleRequest(req, res)
})
this._codeServerMain.handleRequest(req, res)
}

wsRouter.ws("/", ensureAuthenticated, (req) => {
codeServerMain.handleUpgrade(req, req.socket)
private $proxyWebsocket = async (req: WebsocketRequest) => {
this._codeServerMain.handleUpgrade(req, req.socket)

req.socket.resume()
})
}

//#endregion

/**
* Fetches a code server instance asynchronously to avoid an initial memory overhead.
*/
private ensureCodeServerLoaded: express.Handler = async (req, _res, next) => {
if (this._codeServerMain) {
return next()
}

const { args } = req

/**
* @file ../../../vendor/modules/code-oss-dev/src/vs/server/main.js
*/
const createVSServer = await loadAMDModule<CodeServerLib.CreateServer>(
"vs/server/remoteExtensionHostAgent",
"createServer",
)

try {
this._codeServerMain = await createVSServer(null, {
connectionToken: "0000",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ What's the connectionToken used for again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Connection token is used by VS Code as a basic form of client-side security. At the moment we’ve disabled its usage, but I think it’s worth looking at again as upstream matures the feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: #4479

...args,
// For some reason VS Code takes the port as a string.
port: args.port?.toString(),
})
} catch (createServerError) {
logError(logger, "CodeServerRouteWrapper", createServerError)

const loggedError = isDevMode ? new Error("VS Code may still be compiling...") : createServerError

return next(loggedError)
}

return next()
}

constructor() {
this.router.get("/", this.ensureCodeServerLoaded, this.$root)
this.router.all("*", ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyRequest)
this._wsRouterWrapper.ws("/", ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyWebsocket)
}

return {
router,
wsRouter,
codeServerMain,
dispose() {
this._codeServerMain?.dispose()
}
}
2 changes: 1 addition & 1 deletion vendor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
"postinstall": "./postinstall.sh"
},
"devDependencies": {
"code-oss-dev": "cdr/vscode#d62e8db202f80db7a42233cd56d04e6806109fb1"
"code-oss-dev": "cdr/vscode#8db6c9bb0bc065bdb905dc076f4d4234f126aff7"
}
}
4 changes: 2 additions & 2 deletions vendor/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ clone-response@^1.0.2:
dependencies:
mimic-response "^1.0.0"

code-oss-dev@cdr/vscode#d62e8db202f80db7a42233cd56d04e6806109fb1:
code-oss-dev@cdr/vscode#8db6c9bb0bc065bdb905dc076f4d4234f126aff7:
version "1.61.1"
resolved "https://codeload.github.com/cdr/vscode/tar.gz/d62e8db202f80db7a42233cd56d04e6806109fb1"
resolved "https://codeload.github.com/cdr/vscode/tar.gz/8db6c9bb0bc065bdb905dc076f4d4234f126aff7"
dependencies:
"@microsoft/applicationinsights-web" "^2.6.4"
"@vscode/sqlite3" "4.0.12"
Expand Down