Skip to content

Commit 5f41065

Browse files
committed
addressing greptile comments
1 parent 9d8e720 commit 5f41065

File tree

3 files changed

+55
-49
lines changed

3 files changed

+55
-49
lines changed

src/mcp/resources.ts

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -46,36 +46,6 @@ export function clearAllScreenshots() {
4646
sessionIdToScreenshotNames.clear();
4747
}
4848

49-
/**
50-
* Retry wrapper for clearing screenshots for a session. Uses exponential backoff.
51-
* This protects against transient failures in any consumer of the map.
52-
*/
53-
export async function retryClearScreenshotsForSession(
54-
sessionId: string,
55-
options: { retries?: number; initialDelayMs?: number } = {},
56-
): Promise<void> {
57-
const retries = options.retries ?? 3;
58-
const initialDelayMs = options.initialDelayMs ?? 50;
59-
60-
let attempt = 0;
61-
let delay = initialDelayMs;
62-
// Attempt at least once
63-
while (true) {
64-
try {
65-
clearScreenshotsForSession(sessionId);
66-
return;
67-
} catch (err) {
68-
if (attempt >= retries) {
69-
// Give up
70-
throw err instanceof Error ? err : new Error(String(err));
71-
}
72-
await new Promise((r) => setTimeout(r, delay));
73-
attempt += 1;
74-
delay = Math.min(delay * 2, 1000);
75-
}
76-
}
77-
}
78-
7949
/**
8050
* Handle listing resources request
8151
* @returns A list of available resources including screenshots

src/sessionManager.ts

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Page, BrowserContext } from "@browserbasehq/stagehand";
22
import type { Config } from "../config.d.ts";
33
import type { Cookie } from "playwright-core";
44
import { createStagehandInstance } from "./stagehandStore.js";
5-
import { retryClearScreenshotsForSession } from "./mcp/resources.js";
5+
import { clearScreenshotsForSession } from "./mcp/resources.js";
66
import type { BrowserSession } from "./types/types.js";
77

88
// Global state for managing browser sessions
@@ -134,10 +134,18 @@ export async function createNewBrowserSession(
134134
}
135135

136136
// Purge any screenshots associated with both internal and Browserbase IDs
137-
retryClearScreenshotsForSession(newSessionId).catch(() => {});
138-
const bbId = browserbaseSessionId;
139-
if (bbId) {
140-
retryClearScreenshotsForSession(bbId).catch(() => {});
137+
try {
138+
clearScreenshotsForSession(newSessionId);
139+
const bbId = browserbaseSessionId;
140+
if (bbId) {
141+
clearScreenshotsForSession(bbId);
142+
}
143+
} catch (err) {
144+
process.stderr.write(
145+
`[SessionManager] WARN - Failed to clear screenshots on disconnect for ${newSessionId}: ${
146+
err instanceof Error ? err.message : String(err)
147+
}\n`,
148+
);
141149
}
142150
});
143151

@@ -201,10 +209,18 @@ async function closeBrowserGracefully(
201209
`[SessionManager] Successfully closed Stagehand and browser for session: ${sessionIdToLog}\n`,
202210
);
203211
// After close, purge any screenshots associated with both internal and Browserbase IDs
204-
retryClearScreenshotsForSession(sessionIdToLog).catch(() => {});
205-
const bbId = session?.stagehand?.browserbaseSessionID;
206-
if (bbId) {
207-
retryClearScreenshotsForSession(bbId).catch(() => {});
212+
try {
213+
clearScreenshotsForSession(sessionIdToLog);
214+
const bbId = session?.stagehand?.browserbaseSessionID;
215+
if (bbId) {
216+
clearScreenshotsForSession(bbId);
217+
}
218+
} catch (err) {
219+
process.stderr.write(
220+
`[SessionManager] WARN - Failed to clear screenshots after close for ${sessionIdToLog}: ${
221+
err instanceof Error ? err.message : String(err)
222+
}\n`,
223+
);
208224
}
209225
} catch (closeError) {
210226
process.stderr.write(
@@ -350,7 +366,15 @@ export async function cleanupSession(sessionId: string): Promise<void> {
350366
browsers.delete(sessionId);
351367

352368
// Always purge screenshots for this (internal) session id
353-
await retryClearScreenshotsForSession(sessionId).catch(() => {});
369+
try {
370+
clearScreenshotsForSession(sessionId);
371+
} catch (err) {
372+
process.stderr.write(
373+
`[SessionManager] WARN - Failed to clear screenshots during cleanup for ${sessionId}: ${
374+
err instanceof Error ? err.message : String(err)
375+
}\n`,
376+
);
377+
}
354378

355379
// Clear default session reference if this was the default
356380
if (sessionId === defaultSessionId && defaultBrowserSession) {

src/stagehandStore.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { randomUUID } from "crypto";
22
import { Stagehand, Page } from "@browserbasehq/stagehand";
33
import { StagehandSession, CreateSessionParams } from "./types/types.js";
44
import type { Config } from "../config.d.ts";
5-
import { retryClearScreenshotsForSession } from "./mcp/resources.js";
5+
import { clearScreenshotsForSession } from "./mcp/resources.js";
66

77
// Store for all active sessions
88
const store = new Map<string, StagehandSession>();
@@ -112,10 +112,16 @@ export const create = async (
112112
process.stderr.write(`[StagehandStore] Session disconnected: ${id}\n`);
113113
store.delete(id);
114114
// Purge by internal store ID and Browserbase session ID
115-
retryClearScreenshotsForSession(id).catch(() => {});
116-
const bbId = session.metadata?.bbSessionId;
117-
if (bbId) {
118-
retryClearScreenshotsForSession(bbId).catch(() => {});
115+
try {
116+
clearScreenshotsForSession(id);
117+
const bbId = session.metadata?.bbSessionId;
118+
if (bbId) {
119+
clearScreenshotsForSession(bbId);
120+
}
121+
} catch {
122+
process.stderr.write(
123+
`[StagehandStore] Error clearing screenshots for session ${id}\n`,
124+
);
119125
}
120126
};
121127

@@ -166,10 +172,16 @@ export const remove = async (id: string): Promise<void> => {
166172
await session.stagehand.close();
167173
process.stderr.write(`[StagehandStore] Session closed: ${id}\n`);
168174
// Purge by internal store ID and Browserbase session ID
169-
await retryClearScreenshotsForSession(id).catch(() => {});
170-
const bbId = session.metadata?.bbSessionId;
171-
if (bbId) {
172-
await retryClearScreenshotsForSession(bbId).catch(() => {});
175+
try {
176+
clearScreenshotsForSession(id);
177+
const bbId = session.metadata?.bbSessionId;
178+
if (bbId) {
179+
clearScreenshotsForSession(bbId);
180+
}
181+
} catch {
182+
process.stderr.write(
183+
`[StagehandStore] Error clearing screenshots for session ${id}\n`,
184+
);
173185
}
174186
} catch (error) {
175187
process.stderr.write(

0 commit comments

Comments
 (0)