Clearing a false positive: my error guard matched too much

#bugsmash #devchallenge #testing #playwright

Project Overview

I built TestFlow Agent — an open-source tool that turns plain-English test cases (or a live browser recording) into runnable Postman, Playwright, and JMeter tests.

Its live-discovery feature launches a headed Chromium via Playwright to record real traffic. That only works where there's a display, so when someone runs the backend inside Docker (no X server), the launch fails. To make that failure friendly, I added a guard that catches the error and returns a clear 503 — "live discovery needs a headed browser; run the backend natively."

Good idea. Buggy execution.

Bug Fix or Performance Improvement

My guard decided "this is a no-display environment" by string-matching the launch error message. I included has been closed in the match:

The problem: Playwright throws "Target page, context or browser has been closed" for a whole range of unrelated failures (a crashed context, a closed page mid-call, a killed browser). Every one of those would now be misdiagnosed as "you're in Docker, run natively" — pointing the user at completely the wrong problem.

An AI code review (GitHub Copilot) flagged it on the PR, and it was dead right: the heuristic was too broad.

Code

Before (backend/services/discoveryService.js):

if (/XServer|X server|\$DISPLAY|has been closed/i.test(message)) {
  const error = new Error('Live discovery needs a headed browser ... run natively');
  error.statusCode = 503;
  throw error;
}
throw launchError;

After:

// Match only display/X-server markers — NOT generic Playwright phrases like
// "Target page, context or browser has been closed", which are unrelated failures.
if (/XServer|X server|\$DISPLAY/i.test(message)) {
  const error = new Error('Live discovery needs a headed browser ... run natively');
  error.statusCode = 503;
  throw error;
}
throw launchError;

🔗 PR: https://github.com/sshankar07/test-flow-agent/pull/5

I verified both directions with a quick check:

const re = /XServer|X server|\$DISPLAY/i;
re.test('...without having a XServer running. Missing X server or $DISPLAY'); // true  ✅ real no-display error still caught
re.test('Target page, context or browser has been closed');                  // false ✅ unrelated error no longer misfires

My Improvements

The guard now fires only on genuine display/X-server errors — no more false "run it natively" advice when the real cause was something else entirely.
Unrelated Playwright failures once again surface as themselves, instead of being swallowed by a misleading 503.
Bonus from the same review pass: the Docker builds moved from npm install to npm ci (reproducible installs, no accidental lockfile drift).

Best Use of Sentry

Not used in this submission.

Best Use of Google AI

Not applicable — though it's worth noting the bug was caught by an AI code review on the pull request, which is a nice argument for keeping AI reviewers in the loop for exactly this kind of over-broad heuristic.

DEV Community