puppeteer / puppeteer

JavaScript API for Chrome and Firefox
https://pptr.dev
Apache License 2.0
88.8k stars 9.08k forks source link

[Bug]: Lost connection is not detected if no messages are being sent over WebSocket #12219

Open strunkie30 opened 7 months ago

strunkie30 commented 7 months ago

Minimal, reproducible example

const browser = await puppeteer
      .connect({
        browserWSEndpoint: process.env.BROWSER_WS_ENDPOINT,
        protocolTimeout: 5000,
      })
      .catch((err) => {
        console.error('unable to start browser: ', err.error.code);
        this.stop();
      });

When i connect to a puppeteer browser, the sockets are closed after a while. But the browser stays connected.

browser.connected === true

I can reproduce it when I'am closing the sockets of process.env.BROWSER_WS_ENDPOINT that puppeteer is still running and the browser is still connected until I run an action like:

const pages = await this.browser.pages();
await pages[0].reload();

This scripts throws the error:

TargetCloseError: Protocol error (Page.reload): Target closed
    at CallbackRegistry.clear (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/common/CallbackRegistry.ts:90:30)
    at CdpCDPSession._onClosed (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/cdp/CDPSession.ts:149:21)
    at Connection.#onClose (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/cdp/Connection.ts:205:15)
    at WebSocket.<anonymous> (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/node/NodeWebSocketTransport.ts:50:22)
    at callListener (/Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/ws@8.16.0/node_modules/ws/lib/event-target.js:290:14)
    at WebSocket.onClose (/Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/ws@8.16.0/node_modules/ws/lib/event-target.js:220:9)
    at WebSocket.emit (node:events:518:28)
    at WebSocket.emitClose (/Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/ws@8.16.0/node_modules/ws/lib/websocket.js:265:10)
    at Socket.socketOnClose (/Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/ws@8.16.0/node_modules/ws/lib/websocket.js:1289:15)
    at Socket.emit (node:events:518:28) {
  cause: ProtocolError: 
      at Callback.<instance_members_initializer> (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/common/CallbackRegistry.ts:114:12)
      at new Callback (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/common/CallbackRegistry.ts:119:3)
      at CallbackRegistry.create (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/common/CallbackRegistry.ts:27:22)
      at Connection._rawSend (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/cdp/Connection.ts:120:22)
      at CdpCDPSession.send (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/cdp/CDPSession.ts:95:29)
      at CdpPage.reload (file:///Users/kevinmeijer/projecten/signeditor-mono/node_modules/.pnpm/puppeteer-core@22.6.2/node_modules/puppeteer-core/src/cdp/Page.ts:923:33)
      at Timeout._onTimeout (file:///Users/kevinmeijer/projecten/signeditor-mono/apps/convert-3d-to-image/src/lib/Cluster/Browser.ts:59:24)
      at runNextTicks (node:internal/process/task_queues:60:5)
      at listOnTimeout (node:internal/timers:540:9)
      at processTimers (node:internal/timers:514:7)
}


### Error string

no error

### Bug behavior

- [ ] Flaky
- [ ] PDF

### Background

_No response_

### Expectation

Puppeteer should disconnect the browser and trigger:
`browser.on('disconnected', () => {})`

### Reality

`browser.on('disconnected', () => {})` isn't triggered

### Puppeteer configuration file (if used)

_No response_

### Puppeteer version

22.6.2

### Node version

v20.11.1

### Package manager

npm

### Package manager version

10.2.4

### Operating system

macOS
github-actions[bot] commented 7 months ago

This issue was not reproducible. Please check that your example runs locally and the following:

Once the above checks are satisfied, please edit your issue with the changes and we will try to reproduce the bug again.


Analyzer run

OrKoN commented 7 months ago

I am not able to reproduce:

import puppeteer from "puppeteer";

const browser = await puppeteer .connect({
    browserWSEndpoint: process.env.BROWSER_WS_ENDPOINT,
    protocolTimeout: 5000,
  });

setInterval(() => {
  console.log(browser.connected)
}, 1000);

starts printing close once the socket connection is closed. Could you provide more details about how exactly you close the socket?

strunkie30 commented 7 months ago

I believe the primary concern is related to the tcp_keepalive_time. If my assumption is correct, Puppeteer does not actively keep the socket alive. Consequently, when we verify if the browser is connected with browser.connected, no message is sent to check the browser's health status.

I think this ticket is related by this: https://github.com/puppeteer/puppeteer/issues/1774

This is a very important issue / feature when you want to keep the browser running for a long time without interacting with it.

OrKoN commented 7 months ago

I see what you mean now. We rely on the close event from the client https://github.com/websockets/ws to update the connection status. It looks the client does not auto-detect broken connections.

OrKoN commented 7 months ago

We should probably implement a keepAlive option on the websocket transports and implement the ping/pong to keep the connection alive as described here https://github.com/websockets/ws?tab=readme-ov-file#how-to-detect-and-close-broken-connections

strunkie30 commented 7 months ago

For now I'am requesting browser.version() every minute and the browser has been running for hours. It would be great if an isAlive feature could be implemented! Thanks, @OrKoN, for your responses!