Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
strunkie30 opened this issue Apr 5, 2024 · 6 comments

Comments

@strunkie30
Copy link

strunkie30 commented Apr 5, 2024

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
Copy link

github-actions bot commented Apr 5, 2024

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

  • Ensure the script does not rely on dependencies outside of puppeteer and puppeteer-core.
  • Ensure the error string is just the error message.
    • Bad:

      Error: something went wrong
        at Object.<anonymous> (/Users/username/repository/script.js:2:1)
        at Module._compile (node:internal/modules/cjs/loader:1159:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
        at Module.load (node:internal/modules/cjs/loader:1037:32)
        at Module._load (node:internal/modules/cjs/loader:878:12)
        at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
        at node:internal/main/run_main_module:23:47
    • Good: Error: something went wrong.

  • Ensure your configuration file (if applicable) is valid.
  • If the issue is flaky (does not reproduce all the time), make sure 'Flaky' is checked.
  • If the issue is not expected to error, make sure to write 'no error'.

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
Copy link
Collaborator

OrKoN commented Apr 5, 2024

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?

@OrKoN OrKoN closed this as not planned Won't fix, can't repro, duplicate, stale Apr 5, 2024
@strunkie30
Copy link
Author

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: #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
Copy link
Collaborator

OrKoN commented Apr 5, 2024

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 OrKoN reopened this Apr 5, 2024
@OrKoN OrKoN changed the title [Bug]: Connection lost is not handled with puppeteer.connect() [Bug]: Lost connection is not detected if no messages are being sent over WebSocket Apr 5, 2024
@OrKoN OrKoN added the P3 label Apr 5, 2024
@OrKoN
Copy link
Collaborator

OrKoN commented Apr 5, 2024

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
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants