From 5a55a7a6fff934476a53f08a7dee0157fbf9ed87 Mon Sep 17 00:00:00 2001 From: Anton Kosyakov Date: Mon, 4 Dec 2023 15:05:11 +0100 Subject: [PATCH] [dashboard] proactively reconnect grpc streams (#19185) --- .../dashboard/src/service/public-api.ts | 21 ++++++-- components/dashboard/src/service/service.tsx | 51 ++++++++----------- .../gitpod-protocol/src/messaging/error.ts | 3 ++ .../src/public-api-converter.spec.ts | 12 +++++ .../src/public-api-converter.ts | 6 +++ 5 files changed, 59 insertions(+), 34 deletions(-) diff --git a/components/dashboard/src/service/public-api.ts b/components/dashboard/src/service/public-api.ts index 186ea57c16d2a0..630add4c0e6146 100644 --- a/components/dashboard/src/service/public-api.ts +++ b/components/dashboard/src/service/public-api.ts @@ -277,16 +277,31 @@ export function stream( try { for await (const response of factory({ signal: abort.signal, + // GCP timeout is 10 minutes, we timeout 3 mins earlier + // to avoid unknown network errors and reconnect gracefully + timeoutMs: 7 * 60 * 1000, })) { backoff = BASE_BACKOFF; cb(response); } } catch (e) { - if (ApplicationError.hasErrorCode(e) && e.code === ErrorCodes.CANCELLED) { + if (abort.signal.aborted) { + // client aborted, don't reconnect, early exit return; } - backoff = Math.min(2 * backoff, MAX_BACKOFF); - console.error("failed to watch prebuild:", e); + if ( + ApplicationError.hasErrorCode(e) && + (e.code === ErrorCodes.DEADLINE_EXCEEDED || + // library aborted: https://github.com/connectrpc/connect-es/issues/954 + // (clean up when fixed, on server abort we should rather backoff with jitter) + e.code === ErrorCodes.CANCELLED) + ) { + // timeout is expected, reconnect with base backoff + backoff = BASE_BACKOFF; + } else { + backoff = Math.min(2 * backoff, MAX_BACKOFF); + console.error(e); + } } const jitter = Math.random() * 0.3 * backoff; const delay = backoff + jitter; diff --git a/components/dashboard/src/service/service.tsx b/components/dashboard/src/service/service.tsx index 9370931036086a..960afd97dda339 100644 --- a/components/dashboard/src/service/service.tsx +++ b/components/dashboard/src/service/service.tsx @@ -13,16 +13,17 @@ import { GitpodServiceImpl, User, WorkspaceInfo, + Disposable, } from "@gitpod/gitpod-protocol"; import { WebSocketConnectionProvider } from "@gitpod/gitpod-protocol/lib/messaging/browser/connection"; import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url"; import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; import { IDEFrontendDashboardService } from "@gitpod/gitpod-protocol/lib/frontend-dashboard-service"; import { RemoteTrackMessage } from "@gitpod/gitpod-protocol/lib/analytics"; -import { helloService, workspaceClient } from "./public-api"; +import { helloService, stream, workspaceClient } from "./public-api"; import { getExperimentsClient } from "../experiments/client"; -import { ConnectError, Code } from "@connectrpc/connect"; import { instrumentWebSocket } from "./metrics"; +import { LotsOfRepliesResponse } from "@gitpod/public-api/lib/gitpod/experimental/v1/dummy_pb"; export const gitpodHostUrl = new GitpodHostUrl(window.location.toString()); @@ -122,11 +123,19 @@ function testPublicAPI(service: any): void { }, }); (async () => { - const MAX_BACKOFF = 60000; - const BASE_BACKOFF = 3000; - let backoff = BASE_BACKOFF; + let previousCount = 0; + const watchLotsOfReplies = () => + stream( + (options) => { + return helloService.lotsOfReplies({ previousCount }, options); + }, + (response) => { + previousCount = response.count; + }, + ); // emulates server side streaming with public API + let watching: Disposable | undefined; while (true) { const isTest = !!user && @@ -135,34 +144,14 @@ function testPublicAPI(service: any): void { gitpodHost: window.location.host, })); if (isTest) { - try { - let previousCount = 0; - for await (const reply of helloService.lotsOfReplies( - { previousCount }, - { - // GCP timeout is 10 minutes, we timeout 3 mins earlier - // to avoid unknown network errors - timeoutMs: 7 * 60 * 1000, - }, - )) { - previousCount = reply.count; - backoff = BASE_BACKOFF; - } - } catch (e) { - if (e instanceof ConnectError && e.code === Code.DeadlineExceeded) { - // timeout is expected, continue as usual - backoff = BASE_BACKOFF; - } else { - backoff = Math.min(2 * backoff, MAX_BACKOFF); - console.error(e); - } + if (!watching) { + watching = watchLotsOfReplies(); } - } else { - backoff = BASE_BACKOFF; + } else if (watching) { + watching.dispose(); + watching = undefined; } - const jitter = Math.random() * 0.3 * backoff; - const delay = backoff + jitter; - await new Promise((resolve) => setTimeout(resolve, delay)); + await new Promise((resolve) => setTimeout(resolve, 3000)); } })(); } diff --git a/components/gitpod-protocol/src/messaging/error.ts b/components/gitpod-protocol/src/messaging/error.ts index bb8e57efecfc9e..451bbdeb4af28b 100644 --- a/components/gitpod-protocol/src/messaging/error.ts +++ b/components/gitpod-protocol/src/messaging/error.ts @@ -113,6 +113,9 @@ export const ErrorCodes = { // 498 The operation was cancelled, typically by the caller. CANCELLED: 498 as const, + // 4981 The deadline expired before the operation could complete. + DEADLINE_EXCEEDED: 4981 as const, + // 500 Internal Server Error INTERNAL_SERVER_ERROR: 500 as const, diff --git a/components/public-api/typescript-common/src/public-api-converter.spec.ts b/components/public-api/typescript-common/src/public-api-converter.spec.ts index a54f01f0f0768b..223365775ac6c8 100644 --- a/components/public-api/typescript-common/src/public-api-converter.spec.ts +++ b/components/public-api/typescript-common/src/public-api-converter.spec.ts @@ -525,6 +525,18 @@ describe("PublicAPIConverter", () => { expect(appError.message).to.equal("cancelled"); }); + it("DEADLINE_EXCEEDED", () => { + const connectError = converter.toError( + new ApplicationError(ErrorCodes.DEADLINE_EXCEEDED, "deadline exceeded"), + ); + expect(connectError.code).to.equal(Code.DeadlineExceeded); + expect(connectError.rawMessage).to.equal("deadline exceeded"); + + const appError = converter.fromError(connectError); + expect(appError.code).to.equal(ErrorCodes.DEADLINE_EXCEEDED); + expect(appError.message).to.equal("deadline exceeded"); + }); + it("INTERNAL_SERVER_ERROR", () => { const connectError = converter.toError( new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "internal server error"), diff --git a/components/public-api/typescript-common/src/public-api-converter.ts b/components/public-api/typescript-common/src/public-api-converter.ts index 9c1f938c269338..4cb926f36cac1c 100644 --- a/components/public-api/typescript-common/src/public-api-converter.ts +++ b/components/public-api/typescript-common/src/public-api-converter.ts @@ -495,6 +495,9 @@ export class PublicAPIConverter { if (reason.code === ErrorCodes.CANCELLED) { return new ConnectError(reason.message, Code.Canceled, undefined, undefined, reason); } + if (reason.code === ErrorCodes.DEADLINE_EXCEEDED) { + return new ConnectError(reason.message, Code.DeadlineExceeded, undefined, undefined, reason); + } if (reason.code === ErrorCodes.INTERNAL_SERVER_ERROR) { return new ConnectError(reason.message, Code.Internal, undefined, undefined, reason); } @@ -561,6 +564,9 @@ export class PublicAPIConverter { if (reason.code === Code.Canceled) { return new ApplicationError(ErrorCodes.CANCELLED, reason.rawMessage); } + if (reason.code === Code.DeadlineExceeded) { + return new ApplicationError(ErrorCodes.DEADLINE_EXCEEDED, reason.rawMessage); + } if (reason.code === Code.Internal) { return new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, reason.rawMessage); }