Skip to content
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

feat: throw new CommitError if an API error occurs during commit process #434

Merged
merged 2 commits into from
Jan 5, 2023

Conversation

chingor13
Copy link
Contributor

@chingor13 chingor13 commented Jan 4, 2023

Towards googleapis/repo-automation-bots#4537

This will allow us to handle this particular class of errors (likely log a warning and ignore). It's easier for us to classify these errors as we throw them rather than trying to get the caller to distinguish which portion of the call to createPullRequest failed.

@chingor13 chingor13 requested a review from a team as a code owner January 4, 2023 22:48
@chingor13 chingor13 requested a review from a team January 4, 2023 22:48
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Jan 4, 2023
cause: Error;
constructor(message: string, cause: Error) {
super(message);
this.cause = cause;
Copy link
Contributor

Choose a reason for hiding this comment

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

The purpose of this is to wrap the original error, so someone catching it upstream can get at the original underlying issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly

logger.info(`Successfully created commit. See commit at ${url}`);
return sha;
} catch (e) {
throw new CommitError(`Error creating commit for: ${treeSha}`, e as Error);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's annoying that TypeScript doesn't type the argument passed to catch, I wish this as could be avoided -- one option would be:

if (e instanceof Error) {
throw new CommitError(...);
} else {
throw e;
}

@chingor13 chingor13 merged commit 5ee7f2a into main Jan 5, 2023
@chingor13 chingor13 deleted the commit-error branch January 5, 2023 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants