-
Notifications
You must be signed in to change notification settings - Fork 444
fix(setup/js): normalize unsafe caught/rejected error property access #43638
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
Changes from all commits
473594a
9ad4141
8fbc62d
4357a3f
31354cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,12 +157,14 @@ async function assertTrustedCheckoutRuntime() { | |
| // A 404 here is ambiguous: it can indicate either a non-user app/bot actor | ||
| // or a real user that is not a collaborator. Disambiguate via users API. | ||
| // Real users resolve via users.getByUsername; app/bot actors return 404. | ||
| if (err.status === 404) { | ||
| const errAny = /** @type {any} */ err; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JSDoc type cast is missing required parentheses: 💡 Detail and fixThe TypeScript JSDoc type-assertion syntax requires wrapping the expression in parentheses: // WRONG — cast is ignored; err is still typed as unknown
const errAny = /** `@type` {any} */ err;
// CORRECT
const errAny = /** `@type` {any} */ (err);Without the parens, The same issue appears at line 166 with const errAny = /** `@type` {any} */ (err);
if (errAny.status === 404) {
const userErrAny = /** `@type` {any} */ (userErr);
if (userErrAny.status === 404) { |
||
| if (errAny.status === 404) { | ||
| try { | ||
| await github.rest.users.getByUsername({ username: actor }); | ||
| throw new Error(`Refusing PR checkout: actor '${actor}' is not a collaborator (requires write or higher)`); | ||
| } catch (userErr) { | ||
| if (userErr.status === 404) { | ||
| const userErrAny = /** @type {any} */ userErr; | ||
| if (userErrAny.status === 404) { | ||
| core.info(`Runtime safety check passed for app actor '${actor}' (not a regular user)`); | ||
| return; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/codebase-design] The
/**@type{any} */ errcast lacks the parentheses required for a JSDoc type assertion in@ts-checkfiles — the correct form is/**@type{any} */ (err). Without parens, the type annotation is a JSDoc comment on theconstdeclaration itself, not a cast expression; TypeScript will still treaterrAnyasunknownand the.statusaccess will fail type-checking.💡 Suggested fix
Note:
error_helpers.cjsuses the correct form at line 28:const errorAsAny = /**@type{any} */ (error);.This pattern is introduced in multiple files in this PR (checkout_pr_branch.cjs, load_experiment_state_from_repo.cjs, run_operation_update_upgrade.cjs). All need the parentheses.
@copilot please address this.