mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-25 20:44:46 -07:00
fix(async): prevent missed async errors from bypassing catch handlers (#13714)
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
This commit is contained in:
@@ -52,5 +52,5 @@ export async function getClientMetadata(): Promise<ClientMetadata> {
|
||||
updateChannel: await getReleaseChannel(__dirname),
|
||||
}))();
|
||||
}
|
||||
return await clientMetadataPromise;
|
||||
return clientMetadataPromise;
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ export async function getExperiments(
|
||||
server: CodeAssistServer,
|
||||
): Promise<Experiments> {
|
||||
if (experimentsPromise) {
|
||||
return await experimentsPromise;
|
||||
return experimentsPromise;
|
||||
}
|
||||
|
||||
experimentsPromise = (async () => {
|
||||
@@ -32,7 +32,7 @@ export async function getExperiments(
|
||||
const response = await server.listExperiments(metadata);
|
||||
return parseExperiments(response);
|
||||
})();
|
||||
return await experimentsPromise;
|
||||
return experimentsPromise;
|
||||
}
|
||||
|
||||
function parseExperiments(response: ListExperimentsResponse): Experiments {
|
||||
|
||||
@@ -546,7 +546,7 @@ async function fetchCachedCredentials(): Promise<
|
||||
> {
|
||||
const useEncryptedStorage = getUseEncryptedStorageFlag();
|
||||
if (useEncryptedStorage) {
|
||||
return await OAuthCredentialStorage.loadCredentials();
|
||||
return OAuthCredentialStorage.loadCredentials();
|
||||
}
|
||||
|
||||
const pathsToTry = [
|
||||
|
||||
@@ -102,10 +102,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(
|
||||
@@ -128,7 +125,7 @@ export class CodeAssistServer implements ContentGenerator {
|
||||
}
|
||||
|
||||
async getCodeAssistGlobalUserSetting(): Promise<CodeAssistGlobalUserSettingResponse> {
|
||||
return await this.requestGet<CodeAssistGlobalUserSettingResponse>(
|
||||
return this.requestGet<CodeAssistGlobalUserSettingResponse>(
|
||||
'getCodeAssistGlobalUserSetting',
|
||||
);
|
||||
}
|
||||
@@ -136,7 +133,7 @@ export class CodeAssistServer implements ContentGenerator {
|
||||
async setCodeAssistGlobalUserSetting(
|
||||
req: SetCodeAssistGlobalUserSettingRequest,
|
||||
): Promise<CodeAssistGlobalUserSettingResponse> {
|
||||
return await this.requestPost<CodeAssistGlobalUserSettingResponse>(
|
||||
return this.requestPost<CodeAssistGlobalUserSettingResponse>(
|
||||
'setCodeAssistGlobalUserSetting',
|
||||
req,
|
||||
);
|
||||
@@ -167,16 +164,13 @@ export class CodeAssistServer implements ContentGenerator {
|
||||
project: projectId,
|
||||
metadata: { ...metadata, duetProject: projectId },
|
||||
};
|
||||
return await this.requestPost<ListExperimentsResponse>(
|
||||
'listExperiments',
|
||||
req,
|
||||
);
|
||||
return this.requestPost<ListExperimentsResponse>('listExperiments', req);
|
||||
}
|
||||
|
||||
async retrieveUserQuota(
|
||||
req: RetrieveUserQuotaRequest,
|
||||
): Promise<RetrieveUserQuotaResponse> {
|
||||
return await this.requestPost<RetrieveUserQuotaResponse>(
|
||||
return this.requestPost<RetrieveUserQuotaResponse>(
|
||||
'retrieveUserQuota',
|
||||
req,
|
||||
);
|
||||
|
||||
@@ -693,7 +693,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,
|
||||
|
||||
@@ -1886,7 +1886,7 @@ describe('GeminiChat', () => {
|
||||
error,
|
||||
);
|
||||
if (shouldRetry) {
|
||||
return await apiCall();
|
||||
return apiCall();
|
||||
}
|
||||
}
|
||||
throw error; // Stop if callback returns false/null or doesn't exist
|
||||
|
||||
@@ -535,7 +535,7 @@ export class GeminiChat {
|
||||
const onPersistent429Callback = async (
|
||||
authType?: string,
|
||||
error?: unknown,
|
||||
) => await handleFallback(this.config, effectiveModel, authType, error);
|
||||
) => handleFallback(this.config, effectiveModel, authType, error);
|
||||
|
||||
const streamResponse = await retryWithBackoff(apiCall, {
|
||||
onPersistent429: onPersistent429Callback,
|
||||
|
||||
@@ -268,7 +268,7 @@ export class HookEventHandler {
|
||||
};
|
||||
|
||||
const context: HookEventContext = { toolName };
|
||||
return await this.executeHooks(HookEventName.BeforeTool, input, context);
|
||||
return this.executeHooks(HookEventName.BeforeTool, input, context);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -288,7 +288,7 @@ export class HookEventHandler {
|
||||
};
|
||||
|
||||
const context: HookEventContext = { toolName };
|
||||
return await this.executeHooks(HookEventName.AfterTool, input, context);
|
||||
return this.executeHooks(HookEventName.AfterTool, input, context);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -301,7 +301,7 @@ export class HookEventHandler {
|
||||
prompt,
|
||||
};
|
||||
|
||||
return await this.executeHooks(HookEventName.BeforeAgent, input);
|
||||
return this.executeHooks(HookEventName.BeforeAgent, input);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -319,7 +319,7 @@ export class HookEventHandler {
|
||||
details,
|
||||
};
|
||||
|
||||
return await this.executeHooks(HookEventName.Notification, input);
|
||||
return this.executeHooks(HookEventName.Notification, input);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -338,7 +338,7 @@ export class HookEventHandler {
|
||||
stop_hook_active: stopHookActive,
|
||||
};
|
||||
|
||||
return await this.executeHooks(HookEventName.AfterAgent, input);
|
||||
return this.executeHooks(HookEventName.AfterAgent, input);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -353,7 +353,7 @@ export class HookEventHandler {
|
||||
};
|
||||
|
||||
const context: HookEventContext = { trigger: source };
|
||||
return await this.executeHooks(HookEventName.SessionStart, input, context);
|
||||
return this.executeHooks(HookEventName.SessionStart, input, context);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -368,7 +368,7 @@ export class HookEventHandler {
|
||||
};
|
||||
|
||||
const context: HookEventContext = { trigger: reason };
|
||||
return await this.executeHooks(HookEventName.SessionEnd, input, context);
|
||||
return this.executeHooks(HookEventName.SessionEnd, input, context);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -383,7 +383,7 @@ export class HookEventHandler {
|
||||
};
|
||||
|
||||
const context: HookEventContext = { trigger };
|
||||
return await this.executeHooks(HookEventName.PreCompress, input, context);
|
||||
return this.executeHooks(HookEventName.PreCompress, input, context);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -398,7 +398,7 @@ export class HookEventHandler {
|
||||
llm_request: defaultHookTranslator.toHookLLMRequest(llmRequest),
|
||||
};
|
||||
|
||||
return await this.executeHooks(HookEventName.BeforeModel, input);
|
||||
return this.executeHooks(HookEventName.BeforeModel, input);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -415,7 +415,7 @@ export class HookEventHandler {
|
||||
llm_response: defaultHookTranslator.toHookLLMResponse(llmResponse),
|
||||
};
|
||||
|
||||
return await this.executeHooks(HookEventName.AfterModel, input);
|
||||
return this.executeHooks(HookEventName.AfterModel, input);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -430,7 +430,7 @@ export class HookEventHandler {
|
||||
llm_request: defaultHookTranslator.toHookLLMRequest(llmRequest),
|
||||
};
|
||||
|
||||
return await this.executeHooks(HookEventName.BeforeToolSelection, input);
|
||||
return this.executeHooks(HookEventName.BeforeToolSelection, input);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -81,7 +81,7 @@ export class HookRunner {
|
||||
this.executeHook(config, eventName, input),
|
||||
);
|
||||
|
||||
return await Promise.all(promises);
|
||||
return Promise.all(promises);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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> {
|
||||
|
||||
@@ -192,7 +192,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;
|
||||
|
||||
@@ -62,7 +62,7 @@ export async function runInDevTraceSpan<R>(
|
||||
const { name: spanName, noAutoEnd, ...restOfSpanOpts } = opts;
|
||||
if (process.env['GEMINI_DEV_TRACING'] !== 'true') {
|
||||
// If GEMINI_DEV_TRACING env var not set, we do not trace.
|
||||
return await fn({
|
||||
return fn({
|
||||
metadata: {
|
||||
name: spanName,
|
||||
attributes: {},
|
||||
@@ -74,66 +74,62 @@ export async function runInDevTraceSpan<R>(
|
||||
}
|
||||
|
||||
const tracer = trace.getTracer(TRACER_NAME, TRACER_VERSION);
|
||||
return await tracer.startActiveSpan(
|
||||
opts.name,
|
||||
restOfSpanOpts,
|
||||
async (span) => {
|
||||
const meta: SpanMetadata = {
|
||||
name: spanName,
|
||||
attributes: {},
|
||||
};
|
||||
const endSpan = () => {
|
||||
try {
|
||||
if (meta.input !== undefined) {
|
||||
span.setAttribute('input-json', safeJsonStringify(meta.input));
|
||||
}
|
||||
if (meta.output !== undefined) {
|
||||
span.setAttribute('output-json', safeJsonStringify(meta.output));
|
||||
}
|
||||
for (const [key, value] of Object.entries(meta.attributes)) {
|
||||
span.setAttribute(key, value as AttributeValue);
|
||||
}
|
||||
if (meta.error) {
|
||||
span.setStatus({
|
||||
code: SpanStatusCode.ERROR,
|
||||
message: getErrorMessage(meta.error),
|
||||
});
|
||||
if (meta.error instanceof Error) {
|
||||
span.recordException(meta.error);
|
||||
}
|
||||
} else {
|
||||
span.setStatus({ code: SpanStatusCode.OK });
|
||||
}
|
||||
} catch (e) {
|
||||
// Log the error but don't rethrow, to ensure span.end() is called.
|
||||
diag.error('Error setting span attributes in endSpan', e);
|
||||
return tracer.startActiveSpan(opts.name, restOfSpanOpts, async (span) => {
|
||||
const meta: SpanMetadata = {
|
||||
name: spanName,
|
||||
attributes: {},
|
||||
};
|
||||
const endSpan = () => {
|
||||
try {
|
||||
if (meta.input !== undefined) {
|
||||
span.setAttribute('input-json', safeJsonStringify(meta.input));
|
||||
}
|
||||
if (meta.output !== undefined) {
|
||||
span.setAttribute('output-json', safeJsonStringify(meta.output));
|
||||
}
|
||||
for (const [key, value] of Object.entries(meta.attributes)) {
|
||||
span.setAttribute(key, value as AttributeValue);
|
||||
}
|
||||
if (meta.error) {
|
||||
span.setStatus({
|
||||
code: SpanStatusCode.ERROR,
|
||||
message: `Error in endSpan: ${getErrorMessage(e)}`,
|
||||
message: getErrorMessage(meta.error),
|
||||
});
|
||||
} finally {
|
||||
span.end();
|
||||
if (meta.error instanceof Error) {
|
||||
span.recordException(meta.error);
|
||||
}
|
||||
} else {
|
||||
span.setStatus({ code: SpanStatusCode.OK });
|
||||
}
|
||||
};
|
||||
try {
|
||||
return await fn({ metadata: meta, endSpan });
|
||||
} catch (e) {
|
||||
meta.error = e;
|
||||
if (noAutoEnd) {
|
||||
// For streaming operations, the delegated endSpan call will not be reached
|
||||
// on an exception, so we must end the span here to prevent a leak.
|
||||
endSpan();
|
||||
}
|
||||
throw e;
|
||||
// Log the error but don't rethrow, to ensure span.end() is called.
|
||||
diag.error('Error setting span attributes in endSpan', e);
|
||||
span.setStatus({
|
||||
code: SpanStatusCode.ERROR,
|
||||
message: `Error in endSpan: ${getErrorMessage(e)}`,
|
||||
});
|
||||
} finally {
|
||||
if (!noAutoEnd) {
|
||||
// For non-streaming operations, this ensures the span is always closed,
|
||||
// and if an error occurred, it will be recorded correctly by endSpan.
|
||||
endSpan();
|
||||
}
|
||||
span.end();
|
||||
}
|
||||
},
|
||||
);
|
||||
};
|
||||
try {
|
||||
return await fn({ metadata: meta, endSpan });
|
||||
} catch (e) {
|
||||
meta.error = e;
|
||||
if (noAutoEnd) {
|
||||
// For streaming operations, the delegated endSpan call will not be reached
|
||||
// on an exception, so we must end the span here to prevent a leak.
|
||||
endSpan();
|
||||
}
|
||||
throw e;
|
||||
} finally {
|
||||
if (!noAutoEnd) {
|
||||
// For non-streaming operations, this ensures the span is always closed,
|
||||
// and if an error occurred, it will be recorded correctly by endSpan.
|
||||
endSpan();
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -966,7 +966,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) {
|
||||
|
||||
@@ -307,7 +307,7 @@ ${textContent}
|
||||
this.config,
|
||||
new WebFetchFallbackAttemptEvent('primary_failed'),
|
||||
);
|
||||
return this.executeFallback(signal);
|
||||
return await this.executeFallback(signal);
|
||||
}
|
||||
|
||||
const sourceListFormatted: string[] = [];
|
||||
|
||||
@@ -436,7 +436,7 @@ export async function loadEnvironmentMemory(
|
||||
`Loading environment memory for trusted root: ${resolvedRoot} (Stopping exactly here)`,
|
||||
);
|
||||
}
|
||||
return await findUpwardGeminiFiles(resolvedRoot, resolvedRoot, debugMode);
|
||||
return findUpwardGeminiFiles(resolvedRoot, resolvedRoot, debugMode);
|
||||
});
|
||||
|
||||
const pathArrays = await Promise.all(traversalPromises);
|
||||
|
||||
Reference in New Issue
Block a user