Skip to content

[server] fix more revealing BB error logs #20402

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.

Sign up for GitHub

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 6 commits into from
Nov 30, 2024
Merged

[server] fix more revealing BB error logs #20402

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
36f2aac
[server] fix more revealing BB error logs
filiptronicek Nov 29, 2024
521cc54
MOORe
filiptronicek Nov 29, 2024
fc00457
Introduce `BitbucketHttpError` to redact more effectively
filiptronicek Nov 30, 2024
d3081f6
Make it work?
filiptronicek Nov 30, 2024
166c05f
comment fix
filiptronicek Nov 30, 2024
ab649a3
💶
filiptronicek Nov 30, 2024
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
12 changes: 10 additions & 2 deletions components/server/src/bitbucket-server/bitbucket-server-context-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ 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 });
Copy link
Contributor

@mustard-mh mustard-mh Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

span.log is using the original e as well

Copy link
Member Author

@filiptronicek filiptronicek Nov 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's expected, since I'm happy to preserve the details in the log span, just don't want to directly log it in the console for tests.

log.error({ userId: user.id }, "Error parsing Bitbucket context", e);
let error = e;
if (e.name === "HTTPError") {
error = e.status; // we don't want to expose the full error message since it contains credentials
}
log.error({ userId: user.id }, "Error parsing Bitbucket context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -271,7 +275,11 @@ export class BitbucketServerContextParser extends AbstractContextParser implemen
};
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", e);
let error = e;
if (e.name === "HTTPError") {
error = e.status; // we don't want to expose the full error message since it contains credentials
}
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", error);
throw e;
} finally {
span.finish();
Expand Down
24 changes: 20 additions & 4 deletions components/server/src/bitbucket/bitbucket-context-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,11 @@ 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);
let error = e;
if (e.name === "HTTPError") {
error = e.status; // we don't want to expose the full error message since it contains credentials
}
log.error({ userId: user.id }, "Error parsing Bitbucket context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -211,7 +215,11 @@ export class BitbucketContextParser extends AbstractContextParser implements ICo
} as NavigatorContext;
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", e);
let error = e;
if (e.name === "HTTPError") {
error = e.status; // we don't want to expose the full error message since it contains credentials
}
log.error({ userId: user.id }, "Error parsing Bitbucket navigator request context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -272,7 +280,11 @@ export class BitbucketContextParser extends AbstractContextParser implements ICo
};
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket pull request context", e);
let error = e;
if (e.name === "HTTPError") {
error = e.status; // we don't want to expose the full error message since it contains credentials
}
log.error({ userId: user.id }, "Error parsing Bitbucket pull request context", error);
throw e;
} finally {
span.finish();
Expand Down Expand Up @@ -304,7 +316,11 @@ export class BitbucketContextParser extends AbstractContextParser implements ICo
};
} catch (e) {
span.log({ error: e });
log.error({ userId: user.id }, "Error parsing Bitbucket issue context", e);
let error = e;
if (e.name === "HTTPError") {
error = e.status; // we don't want to expose the full error message since it contains credentials
}
log.error({ userId: user.id }, "Error parsing Bitbucket issue context", error);
throw e;
} finally {
span.finish();
Expand Down
6 changes: 6 additions & 0 deletions components/server/src/bitbucket/bitbucket-file-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ export class BitbucketFileProvider implements FileProvider {
);
}

if (err.name === "HTTPError") {
err = err.status; // we don't want to expose the full error message since it contains credentials
}
log.error({ userId: user.id }, err);
throw new Error(`Could not fetch ${path} of repository ${repository.owner}/${repository.name}: ${err}`);
}
Expand All @@ -76,6 +79,9 @@ export class BitbucketFileProvider implements FileProvider {
).data;
return contents as string;
} catch (err) {
if (err.name === "HTTPError") {
err = err.status; // we don't want to expose the full error message since it contains credentials
}
log.debug({ userId: user.id }, err);
}
}
Expand Down
Loading