refactor(browser): make sessionName optional, add selector filter, deduplicate session setup
- sessionName is now optional in open/exec; skips session restore when omitted - add selector param to open: returns outerHTML or textContent of matched element - extract session restore logic into private setupSession() to remove duplication - replace per-exception instanceof checks with single HttpException base class check - embed label into InternalServerErrorException message instead of logging separately - update MCP tool schemas and HTTP DTOs to reflect optional sessionName and new selector - add integration tests: sessionless open/exec, selector, selector+readerMode
This commit is contained in:
@@ -30,7 +30,7 @@ export class BrowserController {
|
||||
@ApiResponse({ status: 404, description: 'Session not found' })
|
||||
@ApiResponse({ status: 500, description: 'Browser automation failed' })
|
||||
open(@Body() dto: OpenDto): Promise<OpenResult> {
|
||||
return this.browserService.open(dto.sessionName, dto.url, dto.readerMode ?? false);
|
||||
return this.browserService.open(dto.sessionName, dto.url, dto.readerMode ?? false, dto.selector);
|
||||
}
|
||||
|
||||
@Post('exec')
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
import {
|
||||
Injectable,
|
||||
BadRequestException,
|
||||
HttpException,
|
||||
InternalServerErrorException,
|
||||
Logger,
|
||||
NotFoundException,
|
||||
} from '@nestjs/common';
|
||||
import { chromium } from 'playwright';
|
||||
import type { BrowserContext } from 'playwright';
|
||||
import { Readability } from '@mozilla/readability';
|
||||
import { JSDOM } from 'jsdom';
|
||||
import { SessionService } from '../session/session.service';
|
||||
@@ -29,12 +30,12 @@ export class BrowserService {
|
||||
private readonly codeExecutor: CodeExecutorService,
|
||||
) {}
|
||||
|
||||
async open(sessionName: string, url: string, readerMode = false): Promise<OpenResult> {
|
||||
private async setupSession(context: BrowserContext, sessionName: string | undefined): Promise<void> {
|
||||
if (!sessionName) return;
|
||||
const session = await this.sessionService.findBySessionName(sessionName);
|
||||
if (!session) {
|
||||
throw new NotFoundException(`Session not found: ${sessionName}`);
|
||||
}
|
||||
|
||||
let cookies: any[];
|
||||
let localStorageData: Record<string, string>;
|
||||
try {
|
||||
@@ -43,25 +44,33 @@ export class BrowserService {
|
||||
} catch (err) {
|
||||
throw new InternalServerErrorException('Failed to deserialize session data', { cause: err });
|
||||
}
|
||||
await context.addCookies(cookies);
|
||||
await context.addInitScript((entries: Record<string, string>) => {
|
||||
for (const [k, v] of Object.entries(entries)) {
|
||||
window.localStorage.setItem(k, v);
|
||||
}
|
||||
}, localStorageData);
|
||||
}
|
||||
|
||||
private rethrow(err: unknown, label: string, operation: string): never {
|
||||
if (err instanceof HttpException) {
|
||||
throw err;
|
||||
}
|
||||
throw new InternalServerErrorException(
|
||||
`[${label}] Browser ${operation} failed: ${(err as Error).message}`,
|
||||
{ cause: err },
|
||||
);
|
||||
}
|
||||
|
||||
async open(sessionName: string | undefined, url: string, readerMode = false, selector?: string): Promise<OpenResult> {
|
||||
const label = sessionName ?? 'anonymous';
|
||||
const browser = await chromium.launch({ headless: true, executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH });
|
||||
try {
|
||||
const context = await browser.newContext();
|
||||
|
||||
// Restore cookies
|
||||
await context.addCookies(cookies);
|
||||
|
||||
await this.setupSession(context, sessionName);
|
||||
const page = await context.newPage();
|
||||
|
||||
// Restore localStorage before the page makes any authenticated requests
|
||||
// by injecting it via init script
|
||||
await context.addInitScript((entries: Record<string, string>) => {
|
||||
for (const [k, v] of Object.entries(entries)) {
|
||||
window.localStorage.setItem(k, v);
|
||||
}
|
||||
}, localStorageData);
|
||||
|
||||
this.logger.log(`[${sessionName}] Opening ${url}`);
|
||||
this.logger.log(`[${label}] Opening ${url}`);
|
||||
await page.goto(url, { waitUntil: 'networkidle' });
|
||||
|
||||
const finalUrl = page.url();
|
||||
@@ -69,7 +78,13 @@ export class BrowserService {
|
||||
const rawHtml = await page.content();
|
||||
|
||||
let content: string;
|
||||
if (readerMode) {
|
||||
if (selector) {
|
||||
const dom = new JSDOM(rawHtml, { url: finalUrl });
|
||||
const el = dom.window.document.querySelector(selector);
|
||||
content = readerMode
|
||||
? (el?.textContent?.replace(/\s+/g, ' ').trim() ?? '')
|
||||
: (el?.outerHTML ?? '');
|
||||
} else if (readerMode) {
|
||||
const dom = new JSDOM(rawHtml, { url: finalUrl });
|
||||
const article = new Readability(dom.window.document).parse();
|
||||
content = article ? article.textContent.replace(/\s+/g, ' ').trim() : rawHtml;
|
||||
@@ -77,76 +92,35 @@ export class BrowserService {
|
||||
content = rawHtml;
|
||||
}
|
||||
|
||||
this.logger.log(`[${sessionName}] Loaded: ${finalUrl} — "${title}"${readerMode ? ' (reader mode)' : ''}`);
|
||||
this.logger.log(`[${label}] Loaded: ${finalUrl} — "${title}"${readerMode ? ' (reader mode)' : ''}${selector ? ` (selector: ${selector})` : ''}`);
|
||||
return { url: finalUrl, title, content };
|
||||
} catch (err) {
|
||||
if (
|
||||
err instanceof BadRequestException ||
|
||||
err instanceof NotFoundException ||
|
||||
err instanceof InternalServerErrorException
|
||||
) {
|
||||
throw err;
|
||||
}
|
||||
this.logger.error(`[${sessionName}] Open failed: ${(err as Error).message}`);
|
||||
throw new InternalServerErrorException(
|
||||
`Browser open failed: ${(err as Error).message}`,
|
||||
{ cause: err },
|
||||
);
|
||||
this.rethrow(err, label, 'open');
|
||||
} finally {
|
||||
await browser.close();
|
||||
}
|
||||
}
|
||||
|
||||
async exec(sessionName: string, code: string, url?: string): Promise<ExecResult> {
|
||||
const session = await this.sessionService.findBySessionName(sessionName);
|
||||
if (!session) {
|
||||
throw new NotFoundException(`Session not found: ${sessionName}`);
|
||||
}
|
||||
|
||||
let cookies: any[];
|
||||
let localStorageData: Record<string, string>;
|
||||
try {
|
||||
cookies = JSON.parse(session.cookies);
|
||||
localStorageData = JSON.parse(session.localStorage);
|
||||
} catch (err) {
|
||||
throw new InternalServerErrorException('Failed to deserialize session data', { cause: err });
|
||||
}
|
||||
|
||||
async exec(sessionName: string | undefined, code: string, url?: string): Promise<ExecResult> {
|
||||
const label = sessionName ?? 'anonymous';
|
||||
const browser = await chromium.launch({ headless: true, executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH });
|
||||
try {
|
||||
const context = await browser.newContext();
|
||||
await context.addCookies(cookies);
|
||||
|
||||
await this.setupSession(context, sessionName);
|
||||
const page = await context.newPage();
|
||||
await context.addInitScript((entries: Record<string, string>) => {
|
||||
for (const [k, v] of Object.entries(entries)) {
|
||||
window.localStorage.setItem(k, v);
|
||||
}
|
||||
}, localStorageData);
|
||||
|
||||
if (url) {
|
||||
this.logger.log(`[${sessionName}] exec: navigating to ${url}`);
|
||||
this.logger.log(`[${label}] exec: navigating to ${url}`);
|
||||
await page.goto(url, { waitUntil: 'networkidle' });
|
||||
}
|
||||
|
||||
this.logger.log(`[${sessionName}] exec: running user code`);
|
||||
this.logger.log(`[${label}] exec: running user code`);
|
||||
const result = await this.codeExecutor.execute(page, context, code);
|
||||
|
||||
this.logger.log(`[${sessionName}] exec: done`);
|
||||
this.logger.log(`[${label}] exec: done`);
|
||||
return result;
|
||||
} catch (err) {
|
||||
if (
|
||||
err instanceof BadRequestException ||
|
||||
err instanceof NotFoundException ||
|
||||
err instanceof InternalServerErrorException
|
||||
) {
|
||||
throw err;
|
||||
}
|
||||
this.logger.error(`[${sessionName}] exec failed: ${(err as Error).message}`);
|
||||
throw new InternalServerErrorException(
|
||||
`Browser exec failed: ${(err as Error).message}`,
|
||||
{ cause: err },
|
||||
);
|
||||
this.rethrow(err, label, 'exec');
|
||||
} finally {
|
||||
await browser.close();
|
||||
}
|
||||
|
||||
@@ -2,12 +2,13 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
|
||||
import { IsOptional, IsString, IsUrl } from 'class-validator';
|
||||
|
||||
export class ExecDto {
|
||||
@ApiProperty({
|
||||
description: 'Session name previously created by POST /login',
|
||||
@ApiPropertyOptional({
|
||||
description: 'Session name previously created by POST /login. If omitted, executes without a stored session.',
|
||||
example: 'test-session-1',
|
||||
})
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
sessionName: string;
|
||||
sessionName?: string;
|
||||
|
||||
@ApiPropertyOptional({
|
||||
description: 'URL to navigate to before executing code. Skipped if omitted.',
|
||||
|
||||
@@ -2,12 +2,13 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
|
||||
import { IsBoolean, IsOptional, IsString, IsUrl } from 'class-validator';
|
||||
|
||||
export class OpenDto {
|
||||
@ApiProperty({
|
||||
description: 'Session name previously created by POST /login',
|
||||
@ApiPropertyOptional({
|
||||
description: 'Session name previously created by POST /login. If omitted, opens the URL without a stored session.',
|
||||
example: 'test-session-1',
|
||||
})
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
sessionName: string;
|
||||
sessionName?: string;
|
||||
|
||||
@ApiProperty({
|
||||
description: 'URL to open with the authenticated session',
|
||||
@@ -23,4 +24,12 @@ export class OpenDto {
|
||||
@IsOptional()
|
||||
@IsBoolean()
|
||||
readerMode?: boolean;
|
||||
|
||||
@ApiPropertyOptional({
|
||||
description: 'CSS selector whose matching element content is returned. When omitted the full page HTML is used.',
|
||||
example: '#main-content',
|
||||
})
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
selector?: string;
|
||||
}
|
||||
|
||||
@@ -189,14 +189,15 @@ export class McpService {
|
||||
{
|
||||
description: 'Open a URL using a stored session and return the page title and content',
|
||||
inputSchema: {
|
||||
sessionName: z.string().describe('Session name to restore cookies and localStorage from'),
|
||||
sessionName: z.string().optional().describe('Session name to restore cookies and localStorage from. Omit to open without a stored session.'),
|
||||
url: z.string().url().describe('URL to navigate to'),
|
||||
readerMode: z.boolean().optional().describe('Extract readable plain text instead of raw HTML'),
|
||||
selector: z.string().optional().describe('CSS selector whose matching element content is returned; applied before readerMode'),
|
||||
},
|
||||
},
|
||||
async ({ sessionName, url, readerMode }) => {
|
||||
async ({ sessionName, url, readerMode, selector }) => {
|
||||
try {
|
||||
const result = await this.browserService.open(sessionName, url, readerMode ?? false);
|
||||
const result = await this.browserService.open(sessionName, url, readerMode ?? false, selector);
|
||||
return { content: [{ type: 'text' as const, text: JSON.stringify(result) }] };
|
||||
} catch (err) {
|
||||
return { isError: true, content: [{ type: 'text' as const, text: (err as Error).message }] };
|
||||
@@ -209,7 +210,7 @@ export class McpService {
|
||||
{
|
||||
description: 'Execute arbitrary Playwright JavaScript with `page` and `context` in scope',
|
||||
inputSchema: {
|
||||
sessionName: z.string().describe('Session name to restore'),
|
||||
sessionName: z.string().optional().describe('Session name to restore. Omit to run without a stored session.'),
|
||||
code: z.string().describe('JavaScript code body to execute (async-safe, may use `page` and `context`)'),
|
||||
url: z.string().url().optional().describe('Optional URL to navigate to before running code'),
|
||||
},
|
||||
|
||||
+10
-1
@@ -1,6 +1,15 @@
|
||||
const mockElement = {
|
||||
outerHTML: '<div id="mock">mock content</div>',
|
||||
textContent: 'mock content',
|
||||
};
|
||||
|
||||
export class JSDOM {
|
||||
constructor(public html: string, public options?: any) {}
|
||||
get window() {
|
||||
return { document: {} };
|
||||
return {
|
||||
document: {
|
||||
querySelector: (_selector: string) => mockElement,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -45,13 +45,6 @@ describe('BrowserController', () => {
|
||||
await request(app.getHttpServer()).post('/open').send({}).expect(400);
|
||||
});
|
||||
|
||||
it('returns 400 when sessionName is missing', async () => {
|
||||
await request(app.getHttpServer())
|
||||
.post('/open')
|
||||
.send({ url: 'https://example.com' })
|
||||
.expect(400);
|
||||
});
|
||||
|
||||
it('returns 400 when url is missing', async () => {
|
||||
await request(app.getHttpServer())
|
||||
.post('/open')
|
||||
@@ -65,6 +58,34 @@ describe('BrowserController', () => {
|
||||
.send({ sessionName: 'no-such-session', url: 'https://example.com' })
|
||||
.expect(404);
|
||||
});
|
||||
|
||||
it('succeeds without a session (sessionless open)', async () => {
|
||||
const res = await request(app.getHttpServer())
|
||||
.post('/open')
|
||||
.send({ url: 'https://example.com' })
|
||||
.expect(201);
|
||||
expect(res.body).toMatchObject({
|
||||
url: expect.any(String),
|
||||
title: expect.any(String),
|
||||
content: expect.any(String),
|
||||
});
|
||||
});
|
||||
|
||||
it('returns only selector content when selector is provided', async () => {
|
||||
const res = await request(app.getHttpServer())
|
||||
.post('/open')
|
||||
.send({ url: 'https://example.com', selector: '#mock' })
|
||||
.expect(201);
|
||||
expect(res.body.content).toBe('<div id="mock">mock content</div>');
|
||||
});
|
||||
|
||||
it('returns selector text content in reader mode', async () => {
|
||||
const res = await request(app.getHttpServer())
|
||||
.post('/open')
|
||||
.send({ url: 'https://example.com', selector: '#mock', readerMode: true })
|
||||
.expect(201);
|
||||
expect(res.body.content).toBe('mock content');
|
||||
});
|
||||
});
|
||||
|
||||
// ── POST /exec ─────────────────────────────────────────────────────────────
|
||||
@@ -94,5 +115,13 @@ describe('BrowserController', () => {
|
||||
.send({ sessionName: 'no-such-session', code: 'return 1;' })
|
||||
.expect(404);
|
||||
});
|
||||
|
||||
it('succeeds without a session (sessionless exec)', async () => {
|
||||
const res = await request(app.getHttpServer())
|
||||
.post('/exec')
|
||||
.send({ code: 'return 42;' })
|
||||
.expect(201);
|
||||
expect(res.body).toEqual({ result: 42 });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user