From 414ee459833e4b19b8bafc552423339b39b3de96 Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Thu, 5 Sep 2024 16:54:48 +0200 Subject: [PATCH] [supervisor] Frontend: re-try + track "checkReady" (#20175) * [ws-proxy] Log whenever we can't connect to an upstream * [protocol] Fix linter error * [supervisor] Frontend: re-connect every 5s + track behavior * [supervisor] Frontend: Guard connection re-try with feature flag "supervisor_check_ready_retry" --- components/dashboard/src/service/service.tsx | 21 +++++ .../src/frontend-dashboard-service.ts | 17 ++++ .../gitpod-protocol/src/util/timeout.spec.ts | 59 ++++++++++++++ .../gitpod-protocol/src/util/timeout.ts | 80 +++++++++++++++++++ components/gitpod-protocol/tsconfig.json | 3 +- .../src/ide/supervisor-service-client.ts | 55 ++++++++++--- components/supervisor/frontend/src/index.ts | 5 +- .../src/shared/frontend-dashboard-service.ts | 16 ++++ components/ws-proxy/pkg/proxy/pass.go | 6 ++ components/ws-proxy/pkg/proxy/routes.go | 15 +++- 10 files changed, 258 insertions(+), 19 deletions(-) create mode 100644 components/gitpod-protocol/src/util/timeout.spec.ts create mode 100644 components/gitpod-protocol/src/util/timeout.ts diff --git a/components/dashboard/src/service/service.tsx b/components/dashboard/src/service/service.tsx index a0cba936c5bb04..957e55c4c252e9 100644 --- a/components/dashboard/src/service/service.tsx +++ b/components/dashboard/src/service/service.tsx @@ -184,6 +184,7 @@ export class IDEFrontendService implements IDEFrontendDashboardService.IServer { private clientWindow: Window, ) { this.processServerInfo(); + this.sendFeatureFlagsUpdate(); window.addEventListener("message", (event: MessageEvent) => { if (IDEFrontendDashboardService.isTrackEventData(event.data)) { this.trackEvent(event.data.msg); @@ -200,6 +201,9 @@ export class IDEFrontendService implements IDEFrontendDashboardService.IServer { if (IDEFrontendDashboardService.isOpenDesktopIDE(event.data)) { this.openDesktopIDE(event.data.url); } + if (IDEFrontendDashboardService.isFeatureFlagsRequestEventData(event.data)) { + this.sendFeatureFlagsUpdate(); + } }); window.addEventListener("unload", () => { if (!this.instanceID) { @@ -376,6 +380,23 @@ export class IDEFrontendService implements IDEFrontendDashboardService.IServer { ); } + private async sendFeatureFlagsUpdate() { + const supervisor_check_ready_retry = await getExperimentsClient().getValueAsync( + "supervisor_check_ready_retry", + false, + { + gitpodHost: gitpodHostUrl.toString(), + }, + ); + this.clientWindow.postMessage( + { + type: "ide-feature-flag-update", + flags: { supervisor_check_ready_retry }, + } as IDEFrontendDashboardService.FeatureFlagsUpdateEventData, + "*", + ); + } + relocate(url: string): void { this.clientWindow.postMessage( { type: "ide-relocate", url } as IDEFrontendDashboardService.RelocateEventData, diff --git a/components/gitpod-protocol/src/frontend-dashboard-service.ts b/components/gitpod-protocol/src/frontend-dashboard-service.ts index d90b7663a95e87..61c6195e4dce2e 100644 --- a/components/gitpod-protocol/src/frontend-dashboard-service.ts +++ b/components/gitpod-protocol/src/frontend-dashboard-service.ts @@ -69,6 +69,15 @@ export namespace IDEFrontendDashboardService { info: Info; } + export interface FeatureFlagsUpdateEventData { + type: "ide-feature-flag-update"; + flags: { supervisor_check_ready_retry: boolean }; + } + + export interface FeatureFlagsRequestEventData { + type: "ide-feature-flag-request"; + } + export interface HeartbeatEventData { type: "ide-heartbeat"; } @@ -101,6 +110,14 @@ export namespace IDEFrontendDashboardService { return obj != null && typeof obj === "object" && obj.type === "ide-info-update"; } + export function isFeatureFlagsUpdateEventData(obj: any): obj is FeatureFlagsUpdateEventData { + return obj != null && typeof obj === "object" && obj.type === "ide-feature-flag-update"; + } + + export function isFeatureFlagsRequestEventData(obj: any): obj is FeatureFlagsRequestEventData { + return obj != null && typeof obj === "object" && obj.type === "ide-feature-flag-request"; + } + export function isHeartbeatEventData(obj: any): obj is HeartbeatEventData { return obj != null && typeof obj === "object" && obj.type === "ide-heartbeat"; } diff --git a/components/gitpod-protocol/src/util/timeout.spec.ts b/components/gitpod-protocol/src/util/timeout.spec.ts new file mode 100644 index 00000000000000..59bc0ce97f7802 --- /dev/null +++ b/components/gitpod-protocol/src/util/timeout.spec.ts @@ -0,0 +1,59 @@ +/** + * Copyright (c) 2020 Gitpod GmbH. All rights reserved. + * Licensed under the GNU Affero General Public License (AGPL). + * See License.AGPL.txt in the project root for license information. + */ + +import * as chai from "chai"; +const expect = chai.expect; +import { suite, test } from "@testdeck/mocha"; +import { Timeout } from "./timeout"; + +@suite() +export class TimeoutSpec { + @test + async testSimpleRun() { + const timeout = new Timeout(1); + timeout.start(); + await timeout.await(); + expect(timeout.signal()?.aborted).to.be.true; + } + + @test + async testSimpleRunNotStarted() { + const timeout = new Timeout(1); + await timeout.await(); + expect(timeout.signal()).to.be.undefined; + } + + @test + async testRestart() { + const timeout = new Timeout(20); + timeout.start(); + await timeout.await(); + expect(timeout.signal()?.aborted).to.be.true; + + timeout.restart(); + expect(timeout.signal()).to.not.be.undefined; + expect(timeout.signal()?.aborted).to.be.false; + await timeout.await(); + expect(timeout.signal()?.aborted).to.be.true; + } + + @test + async testClear() { + const timeout = new Timeout(1000); + timeout.restart(); + timeout.clear(); + expect(timeout.signal()).to.be.undefined; + } + + @test + async testAbortCondition() { + const timeout = new Timeout(1, () => false); // will never trigger abort + timeout.start(); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(timeout.signal()).to.not.be.undefined; + expect(timeout.signal()?.aborted).to.be.false; + } +} diff --git a/components/gitpod-protocol/src/util/timeout.ts b/components/gitpod-protocol/src/util/timeout.ts new file mode 100644 index 00000000000000..9e2fb07008d17f --- /dev/null +++ b/components/gitpod-protocol/src/util/timeout.ts @@ -0,0 +1,80 @@ +/** + * Copyright (c) 2024 Gitpod GmbH. All rights reserved. + * Licensed under the GNU Affero General Public License (AGPL). + * See License.AGPL.txt in the project root for license information. + */ + +/** + * A restartable timeout, based on an AbortController + setTimeout. + * + * Note: When cleared/reset, the AbortController is _NOT_ aborted. + */ +export class Timeout { + private _timer: NodeJS.Timeout | undefined; + private _abortController: AbortController | undefined; + + /** + * @param timeout The timeout in milliseconds. + * @param abortCondition An optional condition evaluated on timeout: If set, the abort is only emitted if it evaluates to true. + */ + constructor(readonly timeout: number, readonly abortCondition?: () => boolean) {} + + /** + * Starts a new timeout (and clears the old one). Identical to `reset`. + */ + public start() { + this.restart(); + } + + /** + * Starts a new timeout (and clears the old one). + */ + public restart() { + this.clear(); + + const abortController = new AbortController(); + this._abortController = abortController; + this._timer = setTimeout(() => { + if (this.abortCondition && !this.abortCondition()) { + return; + } + + abortController.abort(); + }, this.timeout); + } + + /** + * Clears the current timeout. + */ + public clear() { + if (this._timer) { + clearTimeout(this._timer); + this._timer = undefined; + } + if (this._abortController) { + this._abortController = undefined; + } + } + + /** + * Convenience method to await the timeout. + * @returns + */ + public async await(): Promise { + const abortController = this._abortController; + if (!abortController) { + return false; + } + + return new Promise((resolve) => { + abortController.signal.addEventListener("abort", () => resolve(true)); + }); + } + + /** + * @returns The AbortSignal of the current timeout, if one is active. + */ + public signal(): AbortSignal | undefined { + return this._abortController?.signal; + } +} diff --git a/components/gitpod-protocol/tsconfig.json b/components/gitpod-protocol/tsconfig.json index e33bb1ddc5a528..c38232d313fa58 100644 --- a/components/gitpod-protocol/tsconfig.json +++ b/components/gitpod-protocol/tsconfig.json @@ -16,8 +16,7 @@ "es6", "es2018", "dom", - "ES2018.Regexp", - "DOM.AsyncIterable" + "ES2018.Regexp" ], "sourceMap": true, "declaration": true, diff --git a/components/supervisor/frontend/src/ide/supervisor-service-client.ts b/components/supervisor/frontend/src/ide/supervisor-service-client.ts index 60d2aaf4f41098..3b66b853f5a3af 100644 --- a/components/supervisor/frontend/src/ide/supervisor-service-client.ts +++ b/components/supervisor/frontend/src/ide/supervisor-service-client.ts @@ -11,23 +11,17 @@ import { } from "@gitpod/supervisor-api-grpc/lib/status_pb"; import { WorkspaceInfoResponse } from "@gitpod/supervisor-api-grpc/lib/info_pb"; import { workspaceUrl } from "../shared/urls"; +import { FrontendDashboardServiceClient } from "../shared/frontend-dashboard-service"; +import { Timeout } from "@gitpod/gitpod-protocol/lib/util/timeout"; export class SupervisorServiceClient { - private static _instance: SupervisorServiceClient | undefined; - static get(): SupervisorServiceClient { - if (!SupervisorServiceClient._instance) { - SupervisorServiceClient._instance = new SupervisorServiceClient(); - } - return SupervisorServiceClient._instance; - } - readonly supervisorReady = this.checkReady("supervisor"); readonly ideReady = this.supervisorReady.then(() => this.checkReady("ide")); readonly contentReady = Promise.all([this.supervisorReady]).then(() => this.checkReady("content")); readonly getWorkspaceInfoPromise = this.supervisorReady.then(() => this.getWorkspaceInfo()); private _supervisorWillShutdown: Promise | undefined; - private constructor() {} + constructor(readonly serviceClient: FrontendDashboardServiceClient) {} public get supervisorWillShutdown() { if (!this._supervisorWillShutdown) { @@ -84,13 +78,44 @@ export class SupervisorServiceClient { if (kind == "supervisor") { wait = ""; } + + // track whenever a) we are done, or b) we try to connect (again) + const trackCheckReady = (p: { aborted?: boolean }, err?: any): void => { + const props: Record = { + component: "supervisor-frontend", + instanceId: this.serviceClient.latestInfo?.instanceId ?? "", + userId: this.serviceClient.latestInfo?.loggedUserId ?? "", + readyKind: kind, + }; + if (err) { + props.errorName = err.name; + props.errorStack = err.message ?? String(err); + } + + props.aborted = String(!!p.aborted); + props.wait = wait; + + this.serviceClient.trackEvent({ + event: "supervisor_check_ready", + properties: props, + }); + }; + + // setup a timeout, which is meant to re-establish the connection every 5 seconds + let isError = false; + const timeout = new Timeout(5000, () => this.serviceClient.isCheckReadyRetryEnabled()); try { + timeout.restart(); + const wsSupervisorStatusUrl = workspaceUrl.with(() => { return { pathname: "/_supervisor/v1/status/" + kind + wait, }; }); - const response = await fetch(wsSupervisorStatusUrl.toString(), { credentials: "include" }); + const response = await fetch(wsSupervisorStatusUrl.toString(), { + credentials: "include", + signal: timeout.signal(), + }); let result; if (response.ok) { result = await response.json(); @@ -112,6 +137,16 @@ export class SupervisorServiceClient { ); } catch (e) { console.debug(`failed to check whether ${kind} is ready, trying again...`, e); + + // we want to track this kind of errors, as they are on the critical path (of revealing the workspace) + isError = true; + trackCheckReady({ aborted: timeout.signal()?.aborted }, e); + } finally { + if (!isError) { + // make sure we don't track twice in case of an error + trackCheckReady({ aborted: timeout.signal()?.aborted }); + } + timeout.clear(); } return this.checkReady(kind, true); } diff --git a/components/supervisor/frontend/src/index.ts b/components/supervisor/frontend/src/index.ts index abca88ae5744bd..7983cacdd8b395 100644 --- a/components/supervisor/frontend/src/index.ts +++ b/components/supervisor/frontend/src/index.ts @@ -85,9 +85,9 @@ LoadingFrame.load().then(async (loading) => { window.gitpod.encrypt = frontendDashboardServiceClient.encrypt.bind(frontendDashboardServiceClient); window.gitpod.isEncryptedData = frontendDashboardServiceClient.isEncryptedData.bind(frontendDashboardServiceClient); - (async () => { - const supervisorServiceClient = SupervisorServiceClient.get(); + const supervisorServiceClient = new SupervisorServiceClient(frontendDashboardServiceClient); + (async () => { let hideDesktopIde = false; const hideDesktopIdeEventListener = frontendDashboardServiceClient.onOpenBrowserIDE(() => { hideDesktopIdeEventListener.dispose(); @@ -271,7 +271,6 @@ LoadingFrame.load().then(async (loading) => { }); }); } - const supervisorServiceClient = SupervisorServiceClient.get(); if (debugWorkspace) { supervisorServiceClient.supervisorWillShutdown.then(() => { window.open("", "_self")?.close(); diff --git a/components/supervisor/frontend/src/shared/frontend-dashboard-service.ts b/components/supervisor/frontend/src/shared/frontend-dashboard-service.ts index bd9946ddc00e36..8052354da2ed3d 100644 --- a/components/supervisor/frontend/src/shared/frontend-dashboard-service.ts +++ b/components/supervisor/frontend/src/shared/frontend-dashboard-service.ts @@ -26,6 +26,7 @@ export class FrontendDashboardServiceClient implements IDEFrontendDashboardServi private resolveInit!: () => void; private initPromise = new Promise((resolve) => (this.resolveInit = resolve)); + private featureFlags: Partial = {}; private version?: number; @@ -59,7 +60,11 @@ export class FrontendDashboardServiceClient implements IDEFrontendDashboardServi if (IDEFrontendDashboardService.isOpenBrowserIDE(event.data)) { this.onOpenBrowserIDEEmitter.fire(undefined); } + if (IDEFrontendDashboardService.isFeatureFlagsUpdateEventData(event.data)) { + this.featureFlags = event.data.flags; + } }); + this.requestFreshFeatureFlags(); } initialize(): Promise { return this.initPromise; @@ -140,6 +145,17 @@ export class FrontendDashboardServiceClient implements IDEFrontendDashboardServi serverUrl.url.origin, ); } + + requestFreshFeatureFlags(): void { + window.postMessage( + { type: "ide-feature-flag-request" } as IDEFrontendDashboardService.FeatureFlagsRequestEventData, + serverUrl.url.origin, + ); + } + + isCheckReadyRetryEnabled(): boolean { + return !!this.featureFlags.supervisor_check_ready_retry; + } } function isSerializedEncryptedData(obj: any): obj is { iv: string; encrypted: string; tag: string } { diff --git a/components/ws-proxy/pkg/proxy/pass.go b/components/ws-proxy/pkg/proxy/pass.go index a0361c900b06bd..596a1fccd92f7b 100644 --- a/components/ws-proxy/pkg/proxy/pass.go +++ b/components/ws-proxy/pkg/proxy/pass.go @@ -210,6 +210,12 @@ func withHTTPErrorHandler(h http.Handler) proxyPassOpt { } } +func withErrorHandler(h errorHandler) proxyPassOpt { + return func(cfg *proxyPassConfig) { + cfg.ErrorHandler = h + } +} + func createDefaultTransport(config *TransportConfig) *http.Transport { // TODO equivalent of client_max_body_size 2048m; necessary ??? // this is based on http.DefaultTransport, with some values exposed to config diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index 24510f60f44b29..d045885298c31c 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -115,8 +115,15 @@ func installWorkspaceRoutes(r *mux.Router, config *RouteHandlerConfig, ip common }), false) routes.HandleSupervisorFrontendRoute(enableCompression(r).PathPrefix("/_supervisor/frontend")) - routes.HandleDirectSupervisorRoute(r.PathPrefix("/_supervisor/v1/status/supervisor"), false) - routes.HandleDirectSupervisorRoute(r.PathPrefix("/_supervisor/v1/status/ide"), false) + statusErrorHandler := func(rw http.ResponseWriter, req *http.Request, connectErr error) { + log.Infof("status handler: could not connect to backend %s: %s", req.URL.String(), connectErrorToCause(connectErr)) + + rw.WriteHeader(http.StatusBadGateway) + } + + routes.HandleDirectSupervisorRoute(r.PathPrefix("/_supervisor/v1/status/supervisor"), false, withErrorHandler(statusErrorHandler)) + routes.HandleDirectSupervisorRoute(r.PathPrefix("/_supervisor/v1/status/ide"), false, withErrorHandler(statusErrorHandler)) + routes.HandleDirectSupervisorRoute(r.PathPrefix("/_supervisor/v1/status/content"), true, withErrorHandler(statusErrorHandler)) routes.HandleDirectSupervisorRoute(r.PathPrefix("/_supervisor/v1"), true) routes.HandleDirectSupervisorRoute(r.PathPrefix("/_supervisor"), true) @@ -294,7 +301,7 @@ func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewaySe }) } -func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated bool) { +func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated bool, proxyPassOpts ...proxyPassOpt) { r := route.Subrouter() r.Use(logRouteHandlerHandler(fmt.Sprintf("HandleDirectSupervisorRoute (authenticated: %v)", authenticated))) r.Use(ir.Config.CorsHandler) @@ -303,7 +310,7 @@ func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated r.Use(ir.Config.WorkspaceAuthHandler) } - r.NewRoute().HandlerFunc(proxyPass(ir.Config, ir.InfoProvider, workspacePodSupervisorResolver)) + r.NewRoute().HandlerFunc(proxyPass(ir.Config, ir.InfoProvider, workspacePodSupervisorResolver, proxyPassOpts...)) } func (ir *ideRoutes) HandleSupervisorFrontendRoute(route *mux.Route) {