Skip to content

Commit

Permalink
[server] fix more revealing BB error logs (#20402)
Browse files Browse the repository at this point in the history
* [server] fix more revealing BB error logs

* MOORe

* Introduce `BitbucketHttpError` to redact more effectively

* Make it work?

* comment fix

* 💶
  • Loading branch information
filiptronicek authored Nov 30, 2024
1 parent 8054819 commit c4d64c0
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { AbstractContextParser, IContextParser, URLParts } from "../workspace/co
import { URL } from "url";
import { BitbucketServer, BitbucketServerApi } from "./bitbucket-server-api";
import { BitbucketServerTokenHelper } from "./bitbucket-server-token-handler";
import { handleBitbucketError } from "./utils";

const DEFAULT_BRANCH = "master";

Expand Down Expand Up @@ -87,8 +88,9 @@ export class BitbucketServerContextParser extends AbstractContextParser implemen

return await this.handleNavigatorContext(ctx, user, repoKind, host, owner, repoName, more);
} catch (e) {
span.addTags({ contextUrl }).log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket context", e);
const error = e instanceof Error ? handleBitbucketError(e) : e;
span.addTags({ contextUrl }).log({ error });
log.error({ userId: user.id }, "Error parsing Bitbucket context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -270,8 +272,9 @@ export class BitbucketServerContextParser extends AbstractContextParser implemen
repository,
};
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", e);
const error = e instanceof Error ? handleBitbucketError(e) : e;
span.log({ error });
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", error);
throw e;
} finally {
span.finish();
Expand Down
25 changes: 25 additions & 0 deletions components/server/src/bitbucket-server/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* 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.
*/

import { RequestOptions } from "bitbucket/src/plugins/register-endpoints/types";

// For some reason we can't import HTTPError from bitbucket/src/error/types. This is only a subset of the actual class
export abstract class HTTPError extends Error {
public request: RequestOptions | undefined;
}

export function handleBitbucketError(err: Error): Error {
if (err.name !== "HTTPError") {
return err;
}

const httpError = err as HTTPError;
if (httpError.request?.headers.authorization) {
httpError.request.headers.authorization = "<redacted>";
}

return httpError;
}
21 changes: 13 additions & 8 deletions components/server/src/bitbucket/bitbucket-context-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { NotFoundError } from "../errors";
import { AbstractContextParser, IContextParser, IssueContexts } from "../workspace/context-parser";
import { BitbucketApiFactory } from "./bitbucket-api-factory";
import { BitbucketTokenHelper } from "./bitbucket-token-handler";
import { handleBitbucketError } from "../bitbucket-server/utils";

const DEFAULT_BRANCH = "master";

Expand Down Expand Up @@ -103,8 +104,9 @@ export class BitbucketContextParser extends AbstractContextParser implements ICo
}
return await this.handleNavigatorContext(ctx, user, host, owner, repoName);
} catch (e) {
span.addTags({ contextUrl }).log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket context", e);
const error = e instanceof Error ? handleBitbucketError(e) : e;
span.addTags({ contextUrl }).log({ error });
log.error({ userId: user.id }, "Error parsing Bitbucket context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -210,8 +212,9 @@ export class BitbucketContextParser extends AbstractContextParser implements ICo
repository,
} as NavigatorContext;
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", e);
const error = e instanceof Error ? handleBitbucketError(e) : e;
span.log({ error });
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -271,8 +274,9 @@ export class BitbucketContextParser extends AbstractContextParser implements ICo
owner,
};
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket pull request context", e);
const error = e instanceof Error ? handleBitbucketError(e) : e;
span.log({ error });
log.error({ userId: user.id }, "Error parsing Bitbucket pull request context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -303,8 +307,9 @@ export class BitbucketContextParser extends AbstractContextParser implements ICo
localBranch: IssueContexts.toBranchName(user, more.title, more.nr),
};
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket issue context", e);
const error = e instanceof Error ? handleBitbucketError(e) : e;
span.log({ error });
log.error({ userId: user.id }, "Error parsing Bitbucket issue context", error);
throw e;
} finally {
span.finish();
Expand Down
9 changes: 6 additions & 3 deletions components/server/src/bitbucket/bitbucket-file-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
import { inject, injectable } from "inversify";
import { FileProvider, MaybeContent, RevisionNotFoundError } from "../repohost/file-provider";
import { BitbucketApiFactory } from "./bitbucket-api-factory";
import { handleBitbucketError } from "../bitbucket-server/utils";

@injectable()
export class BitbucketFileProvider implements FileProvider {
Expand Down Expand Up @@ -47,13 +48,14 @@ export class BitbucketFileProvider implements FileProvider {

return lastCommit;
} catch (err) {
if (err.status && err.status === 404) {
const error = err instanceof Error ? handleBitbucketError(err) : err;
if (error.status && error.status === 404) {
throw new RevisionNotFoundError(
`File ${path} does not exist in repository ${repository.owner}/${repository.name}`,
);
}

log.error({ userId: user.id }, err);
log.error({ userId: user.id }, error);
throw new Error(`Could not fetch ${path} of repository ${repository.owner}/${repository.name}: ${err}`);
}
}
Expand All @@ -76,7 +78,8 @@ export class BitbucketFileProvider implements FileProvider {
).data;
return contents as string;
} catch (err) {
log.debug({ userId: user.id }, err);
const error = err instanceof Error ? handleBitbucketError(err) : err;
log.debug({ userId: user.id }, error);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { RepoURL } from "../repohost/repo-url";
import { RepositoryProvider } from "../repohost/repository-provider";
import { BitbucketApiFactory } from "./bitbucket-api-factory";
import asyncBatch from "async-batch";
import { handleBitbucketError } from "../bitbucket-server/utils";

@injectable()
export class BitbucketRepositoryProvider implements RepositoryProvider {
Expand Down Expand Up @@ -124,7 +125,8 @@ export class BitbucketRepositoryProvider implements RepositoryProvider {
async hasReadAccess(user: User, owner: string, repo: string): Promise<boolean> {
const api = await this.apiFactory.create(user);
const result = await api.repositories.get({ workspace: owner, repo_slug: repo }).catch((e) => {
console.warn({ userId: user.id }, "hasReadAccess error", { owner, repo, status: e.status });
const error = e instanceof Error ? handleBitbucketError(e) : e;
console.warn({ userId: user.id }, "hasReadAccess error", error, { owner, repo });
return null;
});

Expand Down

0 comments on commit c4d64c0

Please sign in to comment.