Skip to content

Promote prefer-promise-reject-errors to error and fix violations - #11145

Draft
joehan wants to merge 8 commits into
mainfrom
ai-improve-565074829-task-fix-prefer-promise-reject-erro
Draft

joehan wants to merge 8 commits into
mainfrom
ai-improve-565074829-task-fix-prefer-promise-reject-erro

Conversation

@joehan

@joehan joehan commented Sep 23, 2026

Copy link
Copy Markdown
Member

Description

This PR resolves all remaining violations of the ESLint prefer-promise-reject-errors rule and promotes the rule from "warn" to "error" in .eslintrc.js.

Changes Made

  • src/emulator/commandUtils.ts: Pass getError(e) in processKillSignal, and use FirebaseError when child process errors or exits with a signal in runScript.
  • src/emulator/dataconnectEmulator.ts: Reject with a descriptive FirebaseError when failing to connect to the SQL Connect emulator.
  • src/emulator/eventarcEmulator.ts: Reject with a FirebaseError when functions emulator is missing in triggerEventFunction.
  • src/profileReport.ts: Reject readline error with the original Error or wrapped FirebaseError.
  • src/emulator/workQueue.spec.ts: Reject with new Error("job failed") in test.
  • src/throttler/throttler.spec.ts: Reject with new Error("retry") in retry test.
  • src/utils.spec.ts: Add explicit eslint-disable-next-line prefer-promise-reject-errors annotations where tests intentionally test rejection handling with non-Error values ("bar", "fail fast").
  • .eslintrc.js: Promote prefer-promise-reject-errors from "warn" to "error".

Verification

  • npm run build passed completely (build:mcp-apps, tsc, copyfiles).
  • npx mocha passed across all modified/affected spec files (commandUtils.spec.ts, workQueue.spec.ts, profileReport.spec.ts, throttler.spec.ts, utils.spec.ts).
  • ESLint passed on all modified files with 0 errors and 0 warnings.
  • Prettier check passed on all modified files.

Related Bug: b/565074829

@joehan joehan self-assigned this Sep 23, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request configures the ESLint rule prefer-promise-reject-errors to trigger an error instead of a warning, and updates various emulator and utility files to reject promises with proper Error or FirebaseError instances. Feedback on the changes suggests avoiding the as any type assertion in src/profileReport.ts by wrapping non-Error values in a new Error object, and utilizing the existing getError utility in src/emulator/commandUtils.ts to simplify error handling.

Comment thread src/profileReport.ts
Comment thread src/emulator/commandUtils.ts
Comment thread .eslintrc.js Outdated
"no-prototype-builtins": "warn", // TODO(bkendall): remove, allow to error.
"no-useless-escape": "warn", // TODO(bkendall): remove, allow to error.
"prefer-promise-reject-errors": "warn", // TODO(bkendall): remove, allow to error.
"prefer-promise-reject-errors": "error",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just remove this entirely and fall back to the default value

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed prefer-promise-reject-errors completely from .eslintrc.js to fall back to the preset default ("error") in commit 696801bbc. Verified with npm run test:compile, npx mocha, and full-repo npm run lint:quiet.

@joehan
joehan requested a review from bkendall October 1, 2026 23:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants