Skip to content

Bug: sockets are not properly closed #525

@rolznz

Description

@rolznz

When writing jest tests with AI (alby agent skill) the jest tests don't exit by themselves, and the AI hacks it by adding jest --forceExit

npx jest --detectOpenHandles 2>&1 | tail -30

Review the below - might need to just update nostr-tools or there needs to be a fix there or we call close incorrectly on our side.

What AI says:

The open handle is a TCPWRAP — a lingering WebSocket/TCP connection from the nostr-tools relay pool inside
NWCClient.

The bug: NWCClient.close() is synchronous and returns before WebSocket connections are fully terminated.     
                                                                                                               
  Call chain:                                                                                                  
                                                                                                               
  1. NWCClient.close() calls this.pool.close(this.relayUrls) — src/nwc/NWCClient.ts                            
  2. SimplePool.close() calls relay.close() on each relay, then deletes it from the map                        
  3. AbstractRelay.close() (in nostr-tools) calls this.ws?.close() — which only initiates the WebSocket close  
  handshake. The underlying TCP socket stays alive until the server responds with a close frame.               
                                                                                                               
  Because NWCClient.close() is synchronous and ws.close() is non-blocking, the method returns immediately while
   the TCP socket is still open mid-handshake. This is the dangling TCPWRAP handle that prevents Node.js/Jest  
  from exiting.                                                                                                
                                                                                                               
  The fix in @getalby/sdk: Make NWCClient.close() async so it can wait for the WebSocket connections to        
  actually close. Before calling this.pool.close() (which clears the relay map), capture references to each    
  relay's ws, then await their close events:                                                                   
                                                                                                               
  // In NWCClient (src/nwc/NWCClient.ts)                                                                       
  async close(): Promise<void> {                                                                               
    // Grab WebSocket refs before pool.close() clears the relay map                                            
    const closePromises: Promise<void>[] = [];                                                                 
    for (const url of this.relayUrls) {                                                                        
      const relay = this.pool.relays.get(normalizeURL(url));                                                   
      if (relay?.ws && relay.ws.readyState !== WebSocket.CLOSED) {                                             
        closePromises.push(                                                                                    
          new Promise<void>((resolve) => {                                                                     
            relay.ws!.addEventListener("close", () => resolve(), { once: true });                              
          })                                                                                                   
        );                                                                                                     
      }                                                                                                        
    }                                                                                                          
                                                                                                               
    this.pool.close(this.relayUrls);                                                                           
                                                                                                               
    await Promise.all(closePromises);                                                                          
  }                                                                                                            
                                                                                                               
  This is backward-compatible — callers that don't await get the same fire-and-forget behavior as before, while
   callers that do await (like test teardown) can ensure clean shutdown.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions