Skip to content

Commit

Permalink
Merge pull request #7 from stainless-api/sam/sta-2052-trigger-release…
Browse files Browse the repository at this point in the history
…-please-endpoint-fails-due-to-404-returned-by

Check structure of octokit errors instead of via prototype instance
  • Loading branch information
dgellow authored Aug 25, 2023
2 parents bad50cd + dcf382d commit e8b30ed
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 10 deletions.
54 changes: 54 additions & 0 deletions src/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {GraphqlResponseError} from '@octokit/graphql';
import {RequestError} from '@octokit/request-error';
import {RequestError as RequestErrorBody} from '@octokit/types';

Expand Down Expand Up @@ -91,3 +92,56 @@ export class FileNotFoundError extends Error {
this.name = FileNotFoundError.name;
}
}

/**
* Type guard to check if an error is an Octokit RequestError.
*
* This function checks the structure of the error object to determine if it matches
* the shape of a RequestError. It should be favored instead of `instanceof` checks,
* especially in scenarios where the prototype chain might not be reliable, such as when
* dealing with different versions of a package or when the error object might have been
* modified.
*
* @param error The error object to check.
* @returns A boolean indicating whether the error is a RequestError.
*/
export function isOctokitRequestError(error: unknown): error is RequestError {
if (typeof error === 'object' && error !== null) {
const e = error as RequestError;
return (
e.name === 'HttpError' &&
typeof e.status === 'number' &&
typeof e.request === 'object'
);
}
return false;
}

/**
* Type guard to check if an error is an Octokit GraphqlResponseError.
*
* This function checks the structure of the error object to determine if it matches
* the shape of a GraphqlResponseError. It should be favored instead of `instanceof` checks,
* especially in scenarios where the prototype chain might not be reliable, such as when
* dealing with different versions of a package or when the error object might have been
* modified.
*
* @param error The error object to check.
* @returns A boolean indicating whether the error is a GraphqlResponseError.
*/
export function isOctokitGraphqlResponseError(
error: unknown
): error is GraphqlResponseError<unknown> {
if (typeof error === 'object' && error !== null) {
const e = error as GraphqlResponseError<unknown>;
return (
typeof e.request === 'object' &&
typeof e.headers === 'object' &&
typeof e.response === 'object' &&
typeof e.name === 'string' &&
Array.isArray(e.errors) &&
e.data !== undefined
);
}
return false;
}
8 changes: 4 additions & 4 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import {Commit} from './commit';
import {Octokit} from '@octokit/rest';
import {request} from '@octokit/request';
import {graphql} from '@octokit/graphql';
import {RequestError} from '@octokit/request-error';
import {
GitHubAPIError,
DuplicateReleaseError,
FileNotFoundError,
ConfigurationError,
isOctokitRequestError,
} from './errors';

const MAX_ISSUE_BODY_SIZE = 65536;
Expand Down Expand Up @@ -1590,7 +1590,7 @@ export class GitHub {
};
},
e => {
if (e instanceof RequestError) {
if (isOctokitRequestError(e)) {
if (
e.status === 422 &&
GitHubAPIError.parseErrors(e).some(error => {
Expand Down Expand Up @@ -1754,7 +1754,7 @@ export class GitHub {
this.logger.debug(`SHA for branch: ${sha}`);
return sha;
} catch (e) {
if (e instanceof RequestError && e.status === 404) {
if (isOctokitRequestError(e) && e.status === 404) {
this.logger.debug(`Branch: ${branchName} does not exist`);
return undefined;
}
Expand Down Expand Up @@ -2079,7 +2079,7 @@ const wrapAsync = <T extends Array<any>, V>(
if (errorHandler) {
errorHandler(e as GitHubAPIError);
}
if (e instanceof RequestError) {
if (isOctokitRequestError(e)) {
throw new GitHubAPIError(e);
}
throw e;
Expand Down
11 changes: 5 additions & 6 deletions src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import {
DuplicateReleaseError,
FileNotFoundError,
ConfigurationError,
isOctokitRequestError,
isOctokitGraphqlResponseError,
} from './errors';
import {ManifestPlugin} from './plugin';
import {
Expand All @@ -47,8 +49,6 @@ import {
} from './util/pull-request-overflow-handler';
import {signoffCommitMessage} from './util/signoff-commit-message';
import {CommitExclude} from './util/commit-exclude';
import {RequestError} from '@octokit/request-error';
import {GraphqlResponseError} from '@octokit/graphql';

type ExtraJsonFile = {
type: 'json';
Expand Down Expand Up @@ -1185,10 +1185,9 @@ export class Manifest {
//
// Error mentioned here: https://docs.github.com/en/code-security/code-scanning/troubleshooting-code-scanning/resource-not-accessible-by-integration
if (
err &&
((err instanceof RequestError && err.status === 403) ||
(err instanceof GraphqlResponseError &&
err.errors?.find(e => e.type === 'FORBIDDEN')))
(isOctokitRequestError(err) && err.status === 403) ||
(isOctokitGraphqlResponseError(err) &&
err.errors?.find(e => e.type === 'FORBIDDEN'))
) {
await this.throwIfChangesBranchesRaceConditionDetected(pullRequests);
createdReleases = await runReleaseProcess();
Expand Down
69 changes: 69 additions & 0 deletions test/errors/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import {expect} from 'chai';
import {GraphqlResponseError} from '@octokit/graphql';
import {
isOctokitGraphqlResponseError,
isOctokitRequestError,
} from '../../src/errors';
import {RequestError} from '@octokit/request-error';

describe('Errors', () => {
describe('isOctokitRequestError', () => {
it('should return true for valid RequestError', () => {
const error = new RequestError('Not Found', 404, {
request: {method: 'GET', url: '/foo/bar', headers: {}},
headers: {},
});
expect(isOctokitRequestError(error)).to.be.true;
});

it('should return false for invalid RequestError', () => {
const error = {
name: 'SomeOtherError',
status: 500,
request: 'invalid_request_object',
};
expect(isOctokitRequestError(error)).to.be.false;
});
});

describe('isOctokitGraphqlResponseError', () => {
it('should return true for valid GraphqlResponseError', () => {
const error = new GraphqlResponseError(
{
method: 'GET',
url: '/foo/bar',
},
{},
{
data: {},
errors: [
{
type: 'FORBIDDEN',
message: 'Resource not accessible by integration',
path: ['foo'],
extensions: {},
locations: [
{
line: 123,
column: 456,
},
],
},
],
}
);
expect(isOctokitGraphqlResponseError(error)).to.be.true;
});

it('should return false for invalid GraphqlResponseError', () => {
const error = {
request: {},
headers: {},
response: {},
name: 'SomeOtherError',
errors: [],
};
expect(isOctokitGraphqlResponseError(error)).to.be.false;
});
});
});

0 comments on commit e8b30ed

Please sign in to comment.