issue overriding shouldUpgradeWebSocket

[anthonyra]

Describe the bug

When using the @cloudflare/actors framework with WebSocket support, the overridden shouldUpgradeWebSocket() method in a derived Actor class is not being called by the base class's fetch() method, even when all upgrade conditions are met (path matches upgradePath and Upgrade: websocket header is present). Instead, the base class appears to call its own default implementation which returns false, preventing WebSocket upgrades from occurring.

To Reproduce

Steps to reproduce the behavior:

This occurs if you follow the sockets example - https://github.com/cloudflare/actors/tree/main/examples/sockets

1. Create a minimal Actor class extending Actor:

  import { Actor, ActorConfiguration } from '@cloudflare/actors'
  import type { Bindings } from './types'

  export class SocketsActor extends Actor<Bindings> {
      static configuration(request: Request): ActorConfiguration {
          return {
              sockets: {
                  upgradePath: '/ws',
                  autoResponse: {
                      ping: 'ping',
                      pong: 'pong'
                  }
              }
          };
      }

      protected async shouldUpgradeWebSocket(request: Request): Promise<boolean> {
          console.log('shouldUpgradeWebSocket() called'); // This NEVER logs
          return true;
      }

      protected onWebSocketConnect(ws: WebSocket, request: Request) {
          console.log('Socket connected'); // This never executes
      }

      protected onRequest(request: Request): Promise<Response> {
          console.log('onRequest called'); // This ALWAYS executes instead
          return Promise.resolve(Response.json({ message: 'Hello, World!' }));
      }
  }

2. Get the actor instance and call fetch() with a WebSocket upgrade request:

  const actor = SocketsActor.get('test-123');
  const request = new Request('http://fake-host/ws', {
      method: 'GET',
      headers: {
          'Upgrade': 'websocket',
          'Connection': 'Upgrade',
          'Sec-WebSocket-Key': 'test-key',
          'Sec-WebSocket-Version': '13'
      }
  });
  const response = await actor.fetch(request);

3. Observe the logs - shouldUpgradeWebSocket() is never called, and onRequest() is executed instead
4. Response status is 200 OK instead of 101 Switching Protocols

Expected behavior

When a WebSocket upgrade request is made to a path matching upgradePath (e.g., /ws):

1. The base Actor.fetch() method should call the derived class's overridden shouldUpgradeWebSocket() method
2. If shouldUpgradeWebSocket() returns true, it should call onWebSocketUpgrade() and return a 101 Switching Protocols response
3. The onWebSocketConnect() lifecycle hook should fire when the connection is established

Actual behavior

1. The base class's default shouldUpgradeWebSocket() (which returns false) appears to be called instead of the override
2. The WebSocket upgrade never occurs
3. The request falls through to onRequest() which returns 200 OK
4. WebSocket connection fails in the browser with "can't establish a connection"

Workaround

Completely override the fetch() method and manually handle the WebSocket upgrade:

  async fetch(request: Request): Promise<Response> {
      const url = new URL(request.url);
      const upgradeHeader = request.headers.get('Upgrade');

      if (url.pathname === '/ws' && upgradeHeader === 'websocket') {
          const shouldUpgrade = await this.shouldUpgradeWebSocket(request);
          if (shouldUpgrade) {
              return this.onWebSocketUpgrade(request);
          }
      }

      return this.onRequest(request);
  }

This workaround successfully triggers the WebSocket upgrade and results in 101 Switching Protocols.

Logs showing the issue

Without the workaround (broken):

  [SocketsActor] fetch() called - BEFORE super.fetch()
  [SocketsActor] DEBUG: url.pathname = /ws
  [SocketsActor] DEBUG: upgradePath = /ws
  [SocketsActor] DEBUG: pathname === upgradePath? true
  [SocketsActor] DEBUG: Upgrade header = websocket
  [SocketsActor] onRequest called  ← Goes here instead of shouldUpgradeWebSocket
  [wrangler:info] GET /v1/ws 200 OK

With the workaround (working):

  [SocketsActor] fetch() called
  [SocketsActor] url.pathname: /ws
  [SocketsActor] Upgrade header: websocket
  [SocketsActor] ✅ WebSocket upgrade conditions met, calling shouldUpgradeWebSocket()
  [SocketsActor] ========== shouldUpgradeWebSocket() CALLED ==========
  [SocketsActor] Returning: true
  [SocketsActor] shouldUpgradeWebSocket() returned: true
  [SocketsActor] Calling onWebSocketUpgrade()
  Socket connected
  [wrangler:info] GET /v1/ws 101 Switching Protocols

Information (please complete the following information):

• TypeScript: 5.7.3
• Node.js: 22.18.0
• Wrangler: 4.0.0
• @cloudflare/actors Version: 0.0.1-beta.6

Additional context

The issue appears to be related to method binding or polymorphism in the base Actor class. The base class's fetch() method contains this logic:

if (url.pathname === upgradePath || url.pathname.startsWith(`${upgradePath}/`)) {
  const shouldUpgrade = await this.shouldUpgradeWebSocket(request);

  if (shouldUpgrade) {
    return Promise.resolve(this.onWebSocketUpgrade(request));
  }
}

Even though the path matches and the condition evaluates to true, the call to this.shouldUpgradeWebSocket(request) appears to invoke the base class implementation (which returns false) rather than the derived class's override. This prevents the WebSocket upgrade from occurring.

[anthonyra]

PR #91 fixes