Skip to content

Commit 67a9401

Browse files
committed
fix(terminal): cleanup broken tabs and show alerts on terminal errors
- 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
1 parent 7fd0e1e commit 67a9401

File tree

1 file changed

+64
-3
lines changed

1 file changed

+64
-3
lines changed

src/components/terminal/terminalManager.js

Lines changed: 64 additions & 3 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,21 @@ 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
574+
575+
// Close the terminal and remove the tab
538576
this.closeTerminal(terminalId);
577+
578+
// Force remove the tab
579+
try {
580+
terminalFile._skipTerminalCloseConfirm = true;
581+
terminalFile.remove(true);
582+
} catch (removeError) {
583+
console.error("Error removing terminal tab:", removeError);
584+
}
585+
586+
// Show alert for connection error
587+
const errorMessage = error?.message || "Connection lost";
588+
alert(strings["error"], `Terminal connection error: ${errorMessage}`);
539589
};
540590

541591
terminalComponent.onTitleChange = async (title) => {
@@ -624,7 +674,7 @@ class TerminalManager {
624674
* Close a terminal session
625675
* @param {string} terminalId - Terminal ID
626676
*/
627-
closeTerminal(terminalId) {
677+
closeTerminal(terminalId, removeTab = false) {
628678
const terminal = this.terminals.get(terminalId);
629679
if (!terminal) return;
630680

@@ -636,6 +686,7 @@ class TerminalManager {
636686
// Cleanup resize observer
637687
if (terminal.file._resizeObserver) {
638688
terminal.file._resizeObserver.disconnect();
689+
terminal.file._resizeObserver = null;
639690
}
640691

641692
// Cleanup focus handlers
@@ -649,6 +700,16 @@ class TerminalManager {
649700
// Remove from map
650701
this.terminals.delete(terminalId);
651702

703+
// Optionally remove the tab as well
704+
if (removeTab && terminal.file) {
705+
try {
706+
terminal.file._skipTerminalCloseConfirm = true;
707+
terminal.file.remove(true);
708+
} catch (removeError) {
709+
console.error("Error removing terminal tab:", removeError);
710+
}
711+
}
712+
652713
if (this.getAllTerminals().size <= 0) {
653714
Executor.stopService();
654715
}

0 commit comments

Comments
 (0)