Remove redundant else branches (#86)

Else branches are an anti pattern especially if you can easily return from the previous branch. Over time, else branches cause deep nesting and make code unreadable and unmaintainable. Remove elses where possible.
This commit is contained in:
Jaana Dogan 2025-04-21 12:15:47 -07:00 committed by GitHub
parent dea0782c89
commit 53a5728009
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 59 additions and 61 deletions

View File

@ -54,20 +54,21 @@ process.on('unhandledRejection', (reason, _promise) => {
); );
console.warn('-----------------------------------------'); console.warn('-----------------------------------------');
console.warn('Reason:', reason); console.warn('Reason:', reason);
// No process.exit(1); return;
} else { // No process.exit(1); Don't exit.
// Log other unexpected unhandled rejections as critical errors
console.error('=========================================');
console.error('CRITICAL: Unhandled Promise Rejection!');
console.error('=========================================');
console.error('Reason:', reason);
console.error('Stack trace may follow:');
if (!(reason instanceof Error)) {
console.error(reason);
}
// Exit for genuinely unhandled errors
process.exit(1);
} }
// Log other unexpected unhandled rejections as critical errors
console.error('=========================================');
console.error('CRITICAL: Unhandled Promise Rejection!');
console.error('=========================================');
console.error('Reason:', reason);
console.error('Stack trace may follow:');
if (!(reason instanceof Error)) {
console.error(reason);
}
// Exit for genuinely unhandled errors
process.exit(1);
}); });
// --- Global Entry Point --- // --- Global Entry Point ---

View File

@ -70,13 +70,13 @@ export function usePipedInput(): PipedInputState {
stdin.removeListener('error', handleError); stdin.removeListener('error', handleError);
stdin.removeListener('end', handleEnd); stdin.removeListener('end', handleEnd);
}; };
} else {
// No piped input (running interactively)
setIsLoading(false);
// Optionally set an 'info' state or just let isLoading=false & isPiped=false suffice
// setError('No piped input detected.'); // Maybe don't treat this as an 'error'
} }
// No piped input (running interactively)
setIsLoading(false);
// Optionally set an 'info' state or just let isLoading=false & isPiped=false suffice
// setError('No piped input detected.'); // Maybe don't treat this as an 'error'
// Intentionally run only once on mount or when stdin theoretically changes // Intentionally run only once on mount or when stdin theoretically changes
}, [stdin, isRawModeSupported, setRawMode /*, exit */]); }, [stdin, isRawModeSupported, setRawMode /*, exit */]);

View File

@ -129,10 +129,9 @@ export class GeminiClient {
if (error instanceof Error && error.name === 'AbortError') { if (error instanceof Error && error.name === 'AbortError') {
console.log('Gemini stream request aborted by user.'); console.log('Gemini stream request aborted by user.');
throw error; throw error;
} else {
console.error(`Error during Gemini stream or tool interaction:`, error);
throw error;
} }
console.error(`Error during Gemini stream or tool interaction:`, error);
throw error;
} }
} }

View File

@ -152,14 +152,13 @@ export class Turn {
); );
if (confirmationDetails) { if (confirmationDetails) {
return { ...pendingToolCall, confirmationDetails }; return { ...pendingToolCall, confirmationDetails };
} else {
const result = await tool.execute(pendingToolCall.args);
return {
...pendingToolCall,
result,
confirmationDetails: undefined,
};
} }
const result = await tool.execute(pendingToolCall.args);
return {
...pendingToolCall,
result,
confirmationDetails: undefined,
};
} catch (execError: unknown) { } catch (execError: unknown) {
return { return {
...pendingToolCall, ...pendingToolCall,
@ -191,17 +190,17 @@ export class Turn {
type: GeminiEventType.ToolCallConfirmation, type: GeminiEventType.ToolCallConfirmation,
value: serverConfirmationetails, value: serverConfirmationetails,
}; };
} else {
const responsePart = this.buildFunctionResponse(outcome);
this.fnResponses.push(responsePart);
const responseInfo: ToolCallResponseInfo = {
callId: outcome.callId,
responsePart,
resultDisplay: outcome.result?.returnDisplay,
error: outcome.error,
};
yield { type: GeminiEventType.ToolCallResponse, value: responseInfo };
} }
const responsePart = this.buildFunctionResponse(outcome);
this.fnResponses.push(responsePart);
const responseInfo: ToolCallResponseInfo = {
callId: outcome.callId,
responsePart,
resultDisplay: outcome.result?.returnDisplay,
error: outcome.error,
};
yield { type: GeminiEventType.ToolCallResponse, value: responseInfo };
return;
} }
} }
@ -225,23 +224,23 @@ export class Turn {
// Builds the Part array expected by the Google GenAI API // Builds the Part array expected by the Google GenAI API
private buildFunctionResponse(outcome: ServerToolExecutionOutcome): Part { private buildFunctionResponse(outcome: ServerToolExecutionOutcome): Part {
const { name, result, error } = outcome; const { name, result, error } = outcome;
let fnResponsePayload: Record<string, unknown>;
if (error) { if (error) {
// Format error for the LLM // Format error for the LLM
const errorMessage = error?.message || String(error); const errorMessage = error?.message || String(error);
fnResponsePayload = { error: `Tool execution failed: ${errorMessage}` };
console.error(`[Server Turn] Error executing tool ${name}:`, error); console.error(`[Server Turn] Error executing tool ${name}:`, error);
} else { return {
// Pass successful tool result (content meant for LLM) functionResponse: {
fnResponsePayload = { output: result?.llmContent ?? '' }; // Default to empty string if no content name,
id: outcome.callId,
response: { error: `Tool execution failed: ${errorMessage}` },
},
};
} }
return { return {
functionResponse: { functionResponse: {
name, name,
id: outcome.callId, id: outcome.callId,
response: fnResponsePayload, response: { output: result?.llmContent ?? '' },
}, },
}; };
} }

View File

@ -299,32 +299,31 @@ export class BackgroundTerminalAnalyzer {
const { stdout } = await execAsync(command); const { stdout } = await execAsync(command);
// Check if the output contains the process information (it will have the image name if found) // Check if the output contains the process information (it will have the image name if found)
return stdout.toLowerCase().includes('.exe'); // A simple check, adjust if needed return stdout.toLowerCase().includes('.exe'); // A simple check, adjust if needed
} else {
// Linux/macOS/Unix-like: Use kill -0 signal
// process.kill sends signal 0 to check existence without killing
process.kill(pid, 0);
return true; // If no error is thrown, process exists
} }
// Linux/macOS/Unix-like: Use kill -0 signal
// process.kill sends signal 0 to check existence without killing
process.kill(pid, 0);
return true; // If no error is thrown, process exists
} catch (error: unknown) { } catch (error: unknown) {
if (isNodeError(error) && error.code === 'ESRCH') { if (isNodeError(error) && error.code === 'ESRCH') {
// ESRCH: Standard error code for "No such process" on Unix-like systems // ESRCH: Standard error code for "No such process" on Unix-like systems
return false; return false;
} else if ( }
if (
process.platform === 'win32' && process.platform === 'win32' &&
getErrorMessage(error).includes('No tasks are running') getErrorMessage(error).includes('No tasks are running')
) { ) {
// tasklist specific error when PID doesn't exist // tasklist specific error when PID doesn't exist
return false; return false;
} else {
// Other errors (e.g., EPERM - permission denied) mean we couldn't determine status.
// Re-throwing might be appropriate depending on desired behavior.
// Here, we log it and cautiously return true, assuming it *might* still be running.
console.warn(
`isProcessRunning(${pid}) encountered error: ${getErrorMessage(error)}. Assuming process might still exist.`,
);
// Or you could throw the error: throw new Error(`Failed to check process status for PID ${pid}: ${error.message}`);
return true; // Cautious assumption
} }
// Other errors (e.g., EPERM - permission denied) mean we couldn't determine status.
// Re-throwing might be appropriate depending on desired behavior.
// Here, we log it and cautiously return true, assuming it *might* still be running.
console.warn(
`isProcessRunning(${pid}) encountered error: ${getErrorMessage(error)}. Assuming process might still exist.`,
);
// Or you could throw the error: throw new Error(`Failed to check process status for PID ${pid}: ${error.message}`);
return true; // Cautious assumption
} }
} }