[public-api] improve request exception logging

components/dashboard/src/service/metrics.ts

@@ -53,16 +53,19 @@ export function instrumentWebSocket(ws: WebSocket, origin: string) {
 export function reportError(...args: any[]) {
     let err = undefined;
     let details = undefined;
+    let requestContext = undefined;
     if (args[0] instanceof Error) {
         err = args[0];
         details = args[1];
+        requestContext = (args[0] as any)["requestContext"];
     } else if (typeof args[0] === "string") {
         err = new Error(args[0]);
         if (args[1] instanceof Error) {
             err.message += ": " + args[1].message;
             err.name = args[1].name;
             err.stack = args[1].stack;
             details = args[2];
+            requestContext = (args[1] as any)["requestContext"];
         } else if (typeof args[1] === "string") {
             err.message += ": " + args[1];
             details = args[2];
@@ -71,6 +74,10 @@ export function reportError(...args: any[]) {
         }
     }
 
+    if (!err) {
+        return;
+    }
+
     let data = {};
     if (details && typeof details === "object") {
         data = Object.assign(
@@ -90,8 +97,9 @@ export function reportError(...args: any[]) {
             ),
         );
     }
-
-    if (err) {
-        metricsReporter.reportError(err, data);
+    if (requestContext && typeof requestContext === "object") {
+        data = Object.assign(data, requestContext);
     }
+
+    metricsReporter.reportError(err, data);
 }

components/dashboard/src/service/public-api.ts

@@ -48,7 +48,7 @@ const transport = createConnectTransport({
 
 export const converter = new PublicAPIConverter();
 
-export const helloService = createPromiseClient(HelloService, transport);
+export const helloService = createServiceClient(HelloService);
 export const personalAccessTokensService = createPromiseClient(TokensService, transport);
 /**
  * @deprecated use configurationClient instead
@@ -193,19 +193,35 @@ function createServiceClient<T extends ServiceType>(
                 }
                 return jsonRpcOptions.client;
             }
-            /**
-             * The original application error is retained using gRPC metadata to ensure that existing error handling remains intact.
-             */
-            function handleError(e: any): unknown {
-                if (e instanceof ConnectError) {
-                    throw converter.fromError(e);
-                }
-                throw e;
-            }
             return (...args: any[]) => {
+                const requestContext = {
+                    requestMethod: `${type.typeName}/${prop as string}`,
+                };
+                const callOptions: CallOptions = { ...args[1] };
+                const originalOnHeader = callOptions.onHeader;
+                callOptions.onHeader = (headers) => {
+                    if (originalOnHeader) {
+                        originalOnHeader(headers);
+                    }
+                    const requestId = headers.get("x-request-id") || undefined;
+                    if (requestId) {
+                        Object.assign(requestContext, { requestId });
+                    }
+                };
+                args = [args[0], callOptions];
+
+                function handleError(e: any): unknown {
+                    if (e instanceof ConnectError) {
+                        e = converter.fromError(e);
+                    }
+
+                    Object.assign(e, { requestContext });
+                    throw e;
+                }
+
                 const method = type.methods[prop as string];
                 if (!method) {
-                    throw new ConnectError("unimplemented", Code.Unimplemented);
+                    handleError(new ConnectError("unimplemented", Code.Unimplemented));
                 }
 
                 // TODO(ak) default timeouts

components/dashboard/src/service/service.tsx

@@ -103,7 +103,6 @@ function testPublicAPI(service: any): void {
                     try {
                         return await target[propKey](...args);
                     } finally {
-                        const grpcType = "unary";
                         // emulates frequent unary calls to public API
                         const isTest = await getExperimentsClient().getValueAsync(
                             "public_api_dummy_reliability_test",
@@ -114,13 +113,7 @@ function testPublicAPI(service: any): void {
                             },
                         );
                         if (isTest) {
-                            helloService.sayHello({}).catch((e) => {
-                                console.error(e, {
-                                    userId: user?.id,
-                                    workspaceId: args[0],
-                                    grpcType,
-                                });
-                            });
+                            helloService.sayHello({}).catch((e) => console.error(e));
                         }
                     }
                 }
@@ -129,7 +122,6 @@ function testPublicAPI(service: any): void {
         },
     });
     (async () => {
-        const grpcType = "server-stream";
         const MAX_BACKOFF = 60000;
         const BASE_BACKOFF = 3000;
         let backoff = BASE_BACKOFF;
@@ -162,10 +154,7 @@ function testPublicAPI(service: any): void {
                         backoff = BASE_BACKOFF;
                     } else {
                         backoff = Math.min(2 * backoff, MAX_BACKOFF);
-                        console.error(e, {
-                            userId: user?.id,
-                            grpcType,
-                        });
+                        console.error(e);
                     }
                 }
             } else {

components/public-api/typescript-common/README.md

@@ -1,5 +1,13 @@
 # Public API TypeScript Common
 
+## Overview
+
+This package serves as a bridge for code conversion between two distinct Gitpod packages: @gitpod/gitpod-protocol and @gitpod/public-api. Its primary responsibility is to ensure seamless translation of application data structures from the gitpod-protocol format to the public-api gRPC format, and vice versa.
+
+## Allowed Usage
+
+Use this package exclusively for tasks that require data structure from @gitpod/gitpod-protocol and @gitpod/public-api. It's important not to introduce dependencies on this package from gitpod-protocol or public-api to ensure changes in one package don't trigger rebuilds of unrelated components such as ws-manager-bridge and supervisor-frontend.
+
 ## Golden tests
 
 Golden tests are used to test the output of a function against a known good output. The output is stored in a file with the same name as the test file but with a `.golden` extension. The test will fail if the output does not match the golden file.

components/public-api/typescript-common/src/public-api-converter.spec.ts

@@ -367,6 +367,16 @@ describe("PublicAPIConverter", () => {
             expect(appError.message).to.equal("too many running workspaces");
         });
 
+        it("BAD_REQUEST", () => {
+            const connectError = converter.toError(new ApplicationError(ErrorCodes.BAD_REQUEST, "bad request"));
+            expect(connectError.code).to.equal(Code.InvalidArgument);
+            expect(connectError.rawMessage).to.equal("bad request");
+
+            const appError = converter.fromError(connectError);
+            expect(appError.code).to.equal(ErrorCodes.BAD_REQUEST);
+            expect(appError.message).to.equal("bad request");
+        });
+
         it("NOT_FOUND", () => {
             const connectError = converter.toError(new ApplicationError(ErrorCodes.NOT_FOUND, "not found"));
             expect(connectError.code).to.equal(Code.NotFound);
@@ -481,14 +491,15 @@ describe("PublicAPIConverter", () => {
         });
 
         it("Any other error is internal", () => {
-            const error = new Error("unknown");
-            const connectError = converter.toError(error);
+            const reason = new Error("unknown");
+            const connectError = converter.toError(reason);
             expect(connectError.code).to.equal(Code.Internal);
-            expect(connectError.rawMessage).to.equal("unknown");
+            expect(connectError.rawMessage).to.equal("Oops! Something went wrong.");
+            expect(connectError.cause).to.equal(reason);
 
             const appError = converter.fromError(connectError);
             expect(appError.code).to.equal(ErrorCodes.INTERNAL_SERVER_ERROR);
-            expect(appError.message).to.equal("unknown");
+            expect(appError.message).to.equal("Oops! Something went wrong.");
         });
     });
 });

components/public-api/typescript-common/src/public-api-converter.ts

@@ -360,6 +360,9 @@ export class PublicAPIConverter {
                     reason,
                 );
             }
+            if (reason.code === ErrorCodes.BAD_REQUEST) {
+                return new ConnectError(reason.message, Code.InvalidArgument, undefined, undefined, reason);
+            }
             if (reason.code === ErrorCodes.NOT_FOUND) {
                 return new ConnectError(reason.message, Code.NotFound, undefined, undefined, reason);
             }
@@ -386,10 +389,13 @@ export class PublicAPIConverter {
             }
             return new ConnectError(reason.message, Code.Unknown, undefined, undefined, reason);
         }
-        return ConnectError.from(reason, Code.Internal);
+        return new ConnectError(`Oops! Something went wrong.`, Code.Internal, undefined, undefined, reason);
     }
 
     fromError(reason: ConnectError): ApplicationError {
+        if (reason.code === Code.InvalidArgument) {
+            return new ApplicationError(ErrorCodes.BAD_REQUEST, reason.rawMessage);
+        }
         if (reason.code === Code.NotFound) {
             return new ApplicationError(ErrorCodes.NOT_FOUND, reason.rawMessage);
         }

components/server/src/api/server.ts

@@ -147,8 +147,8 @@ export class API {
      * - authentication
      * - server-side observability
      * - logging context
-     * TODO(ak):
      * - rate limitting
+     * TODO(ak):
      * - tracing
      * - cancellation
      */
@@ -167,6 +167,7 @@ export class API {
                         signal: connectContext.signal,
                         headers: connectContext.requestHeader,
                     };
+                    connectContext.responseHeader.set("x-request-id", requestContext.requestId!);
 
                     const withRequestContext = <T>(fn: () => T): T => runWithRequestContext(requestContext, fn);
 
@@ -192,6 +193,9 @@ export class API {
                         grpc_type = "bidi_stream";
                     }
 
+                    const isException = (err: ConnectError) =>
+                        err.code === Code.Internal || err.code === Code.Unknown || err.code === Code.DataLoss;
+
                     grpcServerStarted.labels(grpc_service, grpc_method, grpc_type).inc();
                     const stopTimer = grpcServerHandling.startTimer({ grpc_service, grpc_method, grpc_type });
                     const done = (err?: ConnectError) => {
@@ -201,15 +205,9 @@ export class API {
                         log.debug("public api: done", { grpc_code });
                     };
                     const handleError = (reason: unknown) => {
-                        let err = self.apiConverter.toError(reason);
-                        if (reason != err && err.code === Code.Internal) {
-                            log.error("public api: unexpected internal error", reason);
-                            err = new ConnectError(
-                                `Oops! Something went wrong.`,
-                                Code.Internal,
-                                // pass metadata to preserve the application error
-                                err.metadata,
-                            );
+                        const err = self.apiConverter.toError(reason);
+                        if (isException(err)) {
+                            log.error("public api exception reason:", reason);
                         }
                         done(err);
                         throw err;