Skip to content

[dashboard] proactively reconnect grpc streams #19185

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 1 commit into from
Dec 4, 2023
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
21 changes: 18 additions & 3 deletions components/dashboard/src/service/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,16 +277,31 @@ export function stream<Response>(
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;
Expand Down
51 changes: 20 additions & 31 deletions components/dashboard/src/service/service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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<LotsOfRepliesResponse>(
(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 &&
Expand All @@ -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));
}
})();
}
Expand Down
3 changes: 3 additions & 0 deletions components/gitpod-protocol/src/messaging/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,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);
}
Expand Down Expand Up @@ -450,6 +453,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);
}
Expand Down