Skip to content

Commit 67ca783

Browse files
fix(terminal): cleanup broken tabs and show alerts on terminal errors (#1780)
- Properly dispose and remove failed terminal tabs instead of leaving them stuck - Use alert dialogs instead of toasts for terminal creation/connection errors - Show consolidated error messages for failed session restorations - Fix memory leak in ResizeObserver cleanup * Apply suggestion from @gemini-code-assist[bot] Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 7fd0e1e commit 67ca783

File tree

1 file changed

+57
-4
lines changed

1 file changed

+57
-4
lines changed

src/components/terminal/terminalManager.js

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import TerminalComponent from "./terminal";
88
import "@xterm/xterm/css/xterm.css";
99
import quickTools from "components/quickTools";
1010
import toast from "components/toast";
11+
import alert from "dialogs/alert";
1112
import confirm from "dialogs/confirm";
1213
import openFile from "lib/openFile";
1314
import openFolder from "lib/openFolder";
@@ -108,6 +109,7 @@ class TerminalManager {
108109
const manager = window.editorManager;
109110
const activeFileId = manager?.activeFile?.id;
110111
const restoredTerminals = [];
112+
const failedSessions = [];
111113

112114
for (const session of sessions) {
113115
if (!session?.pid) continue;
@@ -126,10 +128,20 @@ class TerminalManager {
126128
`Failed to restore terminal session ${session.pid}:`,
127129
error,
128130
);
131+
failedSessions.push(session.name || session.pid);
129132
this.removePersistedSession(session.pid);
130133
}
131134
}
132135

136+
// Show alert for failed sessions (don't await to not block UI)
137+
if (failedSessions.length > 0) {
138+
const message =
139+
failedSessions.length === 1
140+
? `Failed to restore terminal: ${failedSessions[0]}`
141+
: `Failed to restore ${failedSessions.length} terminals: ${failedSessions.join(", ")}`;
142+
alert(strings["error"], message);
143+
}
144+
133145
if (activeFileId && manager?.getFile) {
134146
const fileToRestore = manager.getFile(activeFileId, "id");
135147
fileToRestore?.makeActive();
@@ -236,6 +248,32 @@ class TerminalManager {
236248
resolve(instance);
237249
} catch (error) {
238250
console.error("Failed to initialize terminal:", error);
251+
252+
// Cleanup on failure - dispose component and remove broken tab
253+
try {
254+
terminalComponent.dispose();
255+
} catch (disposeError) {
256+
console.error(
257+
"Error disposing terminal component:",
258+
disposeError,
259+
);
260+
}
261+
262+
try {
263+
// Force remove the tab without confirmation
264+
terminalFile._skipTerminalCloseConfirm = true;
265+
terminalFile.remove(true);
266+
} catch (removeError) {
267+
console.error("Error removing terminal tab:", removeError);
268+
}
269+
270+
// Show alert for terminal creation failure
271+
const errorMessage = error?.message || "Unknown error";
272+
alert(
273+
strings["error"],
274+
`Failed to create terminal: ${errorMessage}`,
275+
);
276+
239277
reject(error);
240278
}
241279
}, 100);
@@ -533,9 +571,13 @@ class TerminalManager {
533571

534572
terminalComponent.onError = (error) => {
535573
console.error(`Terminal ${terminalId} error:`, error);
536-
window.toast?.("Terminal connection error");
537-
// Close the terminal tab on error
538-
this.closeTerminal(terminalId);
574+
575+
// Close the terminal and remove the tab
576+
this.closeTerminal(terminalId, true);
577+
578+
// Show alert for connection error
579+
const errorMessage = error?.message || "Connection lost";
580+
alert(strings["error"], `Terminal connection error: ${errorMessage}`);
539581
};
540582

541583
terminalComponent.onTitleChange = async (title) => {
@@ -624,7 +666,7 @@ class TerminalManager {
624666
* Close a terminal session
625667
* @param {string} terminalId - Terminal ID
626668
*/
627-
closeTerminal(terminalId) {
669+
closeTerminal(terminalId, removeTab = false) {
628670
const terminal = this.terminals.get(terminalId);
629671
if (!terminal) return;
630672

@@ -636,6 +678,7 @@ class TerminalManager {
636678
// Cleanup resize observer
637679
if (terminal.file._resizeObserver) {
638680
terminal.file._resizeObserver.disconnect();
681+
terminal.file._resizeObserver = null;
639682
}
640683

641684
// Cleanup focus handlers
@@ -649,6 +692,16 @@ class TerminalManager {
649692
// Remove from map
650693
this.terminals.delete(terminalId);
651694

695+
// Optionally remove the tab as well
696+
if (removeTab && terminal.file) {
697+
try {
698+
terminal.file._skipTerminalCloseConfirm = true;
699+
terminal.file.remove(true);
700+
} catch (removeError) {
701+
console.error("Error removing terminal tab:", removeError);
702+
}
703+
}
704+
652705
if (this.getAllTerminals().size <= 0) {
653706
Executor.stopService();
654707
}

0 commit comments

Comments
 (0)