microsoft / microsoft/playwright
[Feature]: Set `this.name` in `TargetClosedError` constructor
- Dominant language
- TypeScript
- Stars
- 96.3k
- Forks
- 6.5k
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 180
Description
### 🚀 Feature Request
Update: after discussion with @dgozman, we settled on just setting `this.name` in the constructor and not exporting the error to avoid expanding the export scope. This is sufficient for the primary motivation behind this change. Original proposed scope is below, for reference.
---
`TargetClosedError` is currently exported from the internal [`client/errors.ts`](https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/client/errors.ts) module, but it is not part of Playwright’s public package API. Its constructor also does not set `this.name`, so instances created on the client have `name === 'Error'` rather than `'TargetClosedError'`.
The proposed change is to:
1. Export the class as `playwright.errors.TargetClosedError`.
2. Set `this.name = 'TargetClosedError'` in its constructor.
I'd be happy to implement this change if it's approved.
### Example
```ts
try {
await page.click('button');
} catch (error) {
if (error instanceof playwright.errors.TargetClosedError) {
// Handle a page, context, or browser being closed.
}
}
```
### Motivation
Consumers sometimes need to distinguish an expected target closure from other Playwright failures. Without a public class, consumers must inspect error messages, which is brittle, especially in this case, where the message can be overridden.
Playwright already exposes `TimeoutError` through `playwright.errors.TimeoutError` and sets `this.name` for `TimeoutError`s. Exposing `TargetClosedError` through the same API would provide a stable and consistent way to identify this existing error category. Setting its `name` would also preserve the class identity in logs and serialized/deserialized errors.
My primary use here would be for less ambiguous error handling and reporting. I work on an application that uses Playwright to drive browsers for screenshot capturing, and we try to detect browser crash errors, like this `TargetClosedError`, vs. other types, but our only option for doing so is to compare against the string `'Target page, context or browser has been closed'`. This feels brittle, since that string could change in different Playwright versions, or be overwritten by the error's constructor. Being able to compare against the error class would make this more robust. If increasing the export surface is a concern, setting `this.name` in the error's constructor would be almost as good without increasing the export surface.
Contributor guide
Research direction
Start in packages/playwright-core/src/client/errors.ts and inspect the TargetClosedError constructor. Set the instance name to TargetClosedError, without expanding the public export scope. Done means client-created instances report the intended name while the settled scope remains limited to the constructor change.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- testing-qa
- Issue type
- Feature
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100