Skip to content

Commit 895242f

Browse files
committed
[public-api] improve request exception logging
1 parent 3e79652 commit 895242f

File tree

6 files changed

+42
-20
lines changed

6 files changed

+42
-20
lines changed

components/dashboard/src/service/metrics.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ export function reportError(...args: any[]) {
7171
}
7272
}
7373

74+
if (!err) {
75+
return;
76+
}
77+
7478
let data = {};
7579
if (details && typeof details === "object") {
7680
data = Object.assign(
@@ -90,8 +94,9 @@ export function reportError(...args: any[]) {
9094
),
9195
);
9296
}
93-
94-
if (err) {
95-
metricsReporter.reportError(err, data);
97+
if ("requestContext" in err && typeof err["requestContext"] === "object") {
98+
data = Object.assign(data, err["requestContext"]);
9699
}
100+
101+
metricsReporter.reportError(err, data);
97102
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,15 +197,24 @@ function createServiceClient<T extends ServiceType>(
197197
* The original application error is retained using gRPC metadata to ensure that existing error handling remains intact.
198198
*/
199199
function handleError(e: any): unknown {
200+
let requestId = undefined;
200201
if (e instanceof ConnectError) {
201-
throw converter.fromError(e);
202+
requestId = e.metadata.get("x-request-id") || undefined;
203+
e = converter.fromError(e);
202204
}
205+
206+
Object.assign(e, {
207+
requestContext: {
208+
requestId,
209+
requestMethod: `${type.typeName}/${prop as string}`,
210+
},
211+
});
203212
throw e;
204213
}
205214
return (...args: any[]) => {
206215
const method = type.methods[prop as string];
207216
if (!method) {
208-
throw new ConnectError("unimplemented", Code.Unimplemented);
217+
handleError(new ConnectError("unimplemented", Code.Unimplemented));
209218
}
210219

211220
// TODO(ak) default timeouts

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
# Public API TypeScript Common
22

3+
## Overview
4+
5+
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.
6+
7+
## Allowed Usage
8+
9+
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.
10+
311
## Golden tests
412

513
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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -481,14 +481,15 @@ describe("PublicAPIConverter", () => {
481481
});
482482

483483
it("Any other error is internal", () => {
484-
const error = new Error("unknown");
485-
const connectError = converter.toError(error);
484+
const reason = new Error("unknown");
485+
const connectError = converter.toError(reason);
486486
expect(connectError.code).to.equal(Code.Internal);
487-
expect(connectError.rawMessage).to.equal("unknown");
487+
expect(connectError.rawMessage).to.equal("Oops! Something went wrong.");
488+
expect(connectError.cause).to.equal(reason);
488489

489490
const appError = converter.fromError(connectError);
490491
expect(appError.code).to.equal(ErrorCodes.INTERNAL_SERVER_ERROR);
491-
expect(appError.message).to.equal("unknown");
492+
expect(appError.message).to.equal("Oops! Something went wrong.");
492493
});
493494
});
494495
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ export class PublicAPIConverter {
386386
}
387387
return new ConnectError(reason.message, Code.Unknown, undefined, undefined, reason);
388388
}
389-
return ConnectError.from(reason, Code.Internal);
389+
return new ConnectError(`Oops! Something went wrong.`, Code.Internal, undefined, undefined, reason);
390390
}
391391

392392
fromError(reason: ConnectError): ApplicationError {

components/server/src/api/server.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,10 @@ export class API {
159159
get(target, prop) {
160160
return (...args: any[]) => {
161161
const connectContext = args[1] as HandlerContext;
162+
const requestId = connectContext.requestHeader.get("x-request-id") || v4();
163+
connectContext.responseHeader.set("x-request-id", requestId);
162164
const requestContext: RequestContextSeed = {
163-
requestId: v4(),
165+
requestId,
164166
requestKind: "public-api",
165167
requestMethod: `${grpc_service}/${prop as string}`,
166168
startTime: performance.now(),
@@ -192,6 +194,9 @@ export class API {
192194
grpc_type = "bidi_stream";
193195
}
194196

197+
const isException = (err: ConnectError) =>
198+
err.code === Code.Internal || err.code === Code.Unknown || err.code === Code.DataLoss;
199+
195200
grpcServerStarted.labels(grpc_service, grpc_method, grpc_type).inc();
196201
const stopTimer = grpcServerHandling.startTimer({ grpc_service, grpc_method, grpc_type });
197202
const done = (err?: ConnectError) => {
@@ -201,15 +206,9 @@ export class API {
201206
log.debug("public api: done", { grpc_code });
202207
};
203208
const handleError = (reason: unknown) => {
204-
let err = self.apiConverter.toError(reason);
205-
if (reason != err && err.code === Code.Internal) {
206-
log.error("public api: unexpected internal error", reason);
207-
err = new ConnectError(
208-
`Oops! Something went wrong.`,
209-
Code.Internal,
210-
// pass metadata to preserve the application error
211-
err.metadata,
212-
);
209+
const err = self.apiConverter.toError(reason);
210+
if (isException(err)) {
211+
log.error("public api exception reason:", reason);
213212
}
214213
done(err);
215214
throw err;

0 commit comments

Comments
 (0)