Skip to content

fix(async): prevent missed async errors in try/catch with return-await ESLint rule #8168

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+28 −29
Open

fix(async): prevent missed async errors in try/catch with return-await ESLint rule #8168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2ea3f22
build(eslint): add @typescript-eslint/return-await rule
amsminn Sep 10, 2025
94dd8ba
refactor: remove unnecessary return-await in non-try-catch contexts
amsminn Sep 10, 2025
e54ec53
fix: add required return-await in try-catch blocks
amsminn Sep 10, 2025
5151ddd
chore(rettier): apply prettier formatting
amsminn Sep 10, 2025
a33e46d
Merge branch 'main' into refactor/return-await-eslint-rules
amsminn Sep 17, 2025
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
6 changes: 6 additions & 0 deletions eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ export default tseslint.config(
...globals.node,
...globals.es2021,
},
parserOptions: {
projectService: true,
tsconfigRootDir: projectRoot,
},
},
rules: {
// General Best Practice Rules (subset adapted for flat config)
Expand Down Expand Up @@ -159,6 +163,8 @@ export default tseslint.config(
'prefer-const': ['error', { destructuring: 'all' }],
radix: 'error',
'default-case': 'error',
// Prevent redundant async/await patterns
'@typescript-eslint/return-await': ['error', 'in-try-catch'],
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion packages/a2a-server/src/agent/executor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class CoderAgentExecutor implements AgentExecutor {
loadEnvironment(); // Will override any global env with workspace envs
const settings = loadSettings(workspaceRoot);
const extensions = loadExtensions(workspaceRoot);
return await loadConfig(settings, extensions, taskId);
return loadConfig(settings, extensions, taskId);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/mcp/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async function getServerStatus(
server: MCPServerConfig,
): Promise<MCPServerStatus> {
// Test all server types by attempting actual connection
return await testMCPConnection(serverName, server);
return testMCPConnection(serverName, server);
}

export async function listMcpServers(): Promise<void> {
Expand Down
9 changes: 3 additions & 6 deletions packages/cli/src/config/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ export class ExtensionStorage {
}

static async createTmpDir(): Promise<string> {
return await fs.promises.mkdtemp(
path.join(os.tmpdir(), 'gemini-extension'),
);
return fs.promises.mkdtemp(path.join(os.tmpdir(), 'gemini-extension'));
}
}

Expand Down Expand Up @@ -588,7 +586,6 @@ export async function uninstallExtension(
);
manager.remove(extensionName);
const storage = new ExtensionStorage(extensionName);

await fs.promises.rm(storage.getExtensionDir(), {
recursive: true,
force: true,
Expand Down Expand Up @@ -642,7 +639,7 @@ export async function updateExtensionByName(
`Extension "${extensionName}" not found. Run gemini extensions list to see available extensions.`,
);
}
return await updateExtension(extension, cwd, setExtensionUpdateState);
return updateExtension(extension, cwd, setExtensionUpdateState);
}

export async function updateExtension(
Expand Down Expand Up @@ -742,7 +739,7 @@ export async function updateAllUpdatableExtensions(
updateState: Map<string, ExtensionUpdateState>,
) => void,
): Promise<ExtensionUpdateInfo[]> {
return await Promise.all(
return Promise.all(
extensions
.filter(
(extension) =>
Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/zed-integration/acp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class AgentSideConnection implements Client {
* Streams new content to the client including text, tool calls, etc.
*/
async sessionUpdate(params: schema.SessionNotification): Promise<void> {
return await this.#connection.sendNotification(
return this.#connection.sendNotification(
schema.CLIENT_METHODS.session_update,
params,
);
Expand All @@ -83,7 +83,7 @@ export class AgentSideConnection implements Client {
async requestPermission(
params: schema.RequestPermissionRequest,
): Promise<schema.RequestPermissionResponse> {
return await this.#connection.sendRequest(
return this.#connection.sendRequest(
schema.CLIENT_METHODS.session_request_permission,
params,
);
Expand All @@ -92,7 +92,7 @@ export class AgentSideConnection implements Client {
async readTextFile(
params: schema.ReadTextFileRequest,
): Promise<schema.ReadTextFileResponse> {
return await this.#connection.sendRequest(
return this.#connection.sendRequest(
schema.CLIENT_METHODS.fs_read_text_file,
params,
);
Expand All @@ -101,7 +101,7 @@ export class AgentSideConnection implements Client {
async writeTextFile(
params: schema.WriteTextFileRequest,
): Promise<schema.WriteTextFileResponse> {
return await this.#connection.sendRequest(
return this.#connection.sendRequest(
schema.CLIENT_METHODS.fs_write_text_file,
params,
);
Expand Down
9 changes: 3 additions & 6 deletions packages/core/src/code_assist/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,7 @@ export class CodeAssistServer implements ContentGenerator {
async onboardUser(
req: OnboardUserRequest,
): Promise<LongRunningOperationResponse> {
return await this.requestPost<LongRunningOperationResponse>(
'onboardUser',
req,
);
return this.requestPost<LongRunningOperationResponse>('onboardUser', req);
}

async loadCodeAssist(
Expand All @@ -121,15 +118,15 @@ export class CodeAssistServer implements ContentGenerator {
}

async getCodeAssistGlobalUserSetting(): Promise<CodeAssistGlobalUserSettingResponse> {
return await this.requestGet<CodeAssistGlobalUserSettingResponse>(
return this.requestGet<CodeAssistGlobalUserSettingResponse>(
'getCodeAssistGlobalUserSetting',
);
}

async setCodeAssistGlobalUserSetting(
req: SetCodeAssistGlobalUserSettingRequest,
): Promise<CodeAssistGlobalUserSettingResponse> {
return await this.requestPost<CodeAssistGlobalUserSettingResponse>(
return this.requestPost<CodeAssistGlobalUserSettingResponse>(
'setCodeAssistGlobalUserSetting',
req,
);
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/core/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,6 @@ export class GeminiClient {
}
return turn;
}

async generateContent(
contents: Content[],
generationConfig: GenerateContentConfig,
Expand Down Expand Up @@ -608,7 +607,7 @@ export class GeminiClient {
error?: unknown,
) =>
// Pass the captured model to the centralized handler.
await handleFallback(this.config, currentAttemptModel, authType, error);
handleFallback(this.config, currentAttemptModel, authType, error);

const result = await retryWithBackoff(apiCall, {
onPersistent429: onPersistent429Callback,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/core/geminiChat.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ describe('GeminiChat', () => {
error,
);
if (shouldRetry) {
return await apiCall();
return apiCall();
}
}
throw error; // Stop if callback returns false/null or doesn't exist
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/core/geminiChat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export class GeminiChat {
const onPersistent429Callback = async (
authType?: string,
error?: unknown,
) => await handleFallback(this.config, model, authType, error);
) => handleFallback(this.config, model, authType, error);

const streamResponse = await retryWithBackoff(apiCall, {
shouldRetry: (error: unknown) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/mcp/token-storage/hybrid-token-storage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class HybridTokenStorage extends BaseTokenStorage {
}

// Wait for initialization to complete
return await this.storageInitPromise;
return this.storageInitPromise;
}

async getCredentials(serverName: string): Promise<OAuthCredentials | null> {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/services/loopDetectionService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class LoopDetectionService {
this.turnsInCurrentPrompt - this.lastCheckTurn >= this.llmCheckInterval
) {
this.lastCheckTurn = this.turnsInCurrentPrompt;
return await this.checkForLoopWithLLM(signal);
return this.checkForLoopWithLLM(signal);
}

return false;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tools/edit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ Expectation for required parameters:
getFilePath: (params: EditToolParams) => params.file_path,
getCurrentContent: async (params: EditToolParams): Promise<string> => {
try {
return this.config
return await this.config
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm I'm seeing local test failures due to this- can you double check this change?

Copy link
Author

@amsminn amsminn Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

On my local machine, running test didn’t show any test failures related to edit.ts.
However, I did find an issue in editor.ts in the Test (macos-latest, 20.x) test run.

What happened

Before, the try just returned a promise. If that promise later failed, it happened outside the try/catch, so the catch never ran.
With await, the try actually waits, and any error goes straight into the catch. The only issue left was that our catch logged the error but didn’t rethrow it.

Fix

I changed the catch block to log and then rethrow

catch (error) {
  console.error(error);
  throw error;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the detailed explanation, amsminn. Your analysis of how await affects error propagation within try/catch blocks is spot on. It's crucial to await promises within try blocks to ensure that rejections are caught by the catch block, rather than becoming unhandled promise rejections.

Furthermore, your proposed fix of rethrowing the error after logging it in the catch block is indeed the correct and recommended pattern. This ensures that while the error is recorded for debugging, it also continues to propagate up the call stack, allowing higher-level error handlers to address it if necessary. This aligns perfectly with the goal of this pull request to enforce robust async error handling.

.getFileSystemService()
.readTextFile(params.file_path);
} catch (err) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tools/ripGrep.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export async function canUseRipgrep(): Promise<boolean> {
}

await downloadRipGrep(Storage.getGlobalBinDir());
return await fileExists(getRgPath());
return fileExists(getRgPath());
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tools/smart-edit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ A good instruction should concisely answer:
getFilePath: (params: EditToolParams) => params.file_path,
getCurrentContent: async (params: EditToolParams): Promise<string> => {
try {
return this.config
return await this.config
.getFileSystemService()
.readTextFile(params.file_path);
} catch (err) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tools/web-fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ ${textContent}
}

if (processingError) {
return this.executeFallback(signal);
return await this.executeFallback(signal);
}

const sourceListFormatted: string[] = [];
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ export async function openDiff(
return;
}

return new Promise<void>((resolve, reject) => {
return await new Promise<void>((resolve, reject) => {
const childProcess = spawn(diffCommand.command, diffCommand.args, {
stdio: 'inherit',
});
Expand Down