nostrver-se / nostr-php

PHP helper library for Nostr https://nostr-php.dev
https://nostr-php.dev
MIT License
50 stars 15 forks source link

Bug in connection close implementation when sending events #45

Closed atrifat closed 10 months ago

atrifat commented 10 months ago

Issue

$client->close() is not properly supported by some relays. It will trigger connection exception. There is also possibility of bug in phrity/websocket library v1.6.0.

Example:

$private_key = '67dea2ed018072d675f5415ecfaed7d2597555e202d85b3d65ea4e58d2d92ffa'; // private key example which used in test unit

$note = new swentel\nostr\Event\Event();
$note->setContent('Hello world');
$note->setKind(9999);

$signer = new swentel\nostr\Sign\Sign();
$signer->signEvent($note, $private_key);

$eventMessage = new swentel\nostr\Message\EventMessage($note);

// $relayUrl = 'wss://relay.damus.io'; // Can handle $client->close() properly
// $relayUrl = 'wss://nos.lol'; // Can handle $client->close() properly
$relayUrl = 'wss://relay.nostr.band'; // Will give connection error when calling $client->close(). It will be safe if $client->close() replaced by $client->disconnect()
$relayUrl = 'wss://nostr-pub.wellorder.net'; // Same issue with relay.nostr.band

$relay = new swentel\nostr\Relay\Relay($relayUrl, $eventMessage);
$result = $relay->send();

print_r($result);

The following example will show that some relays (wss://relay.nostr.band and wss://nostr-pub.wellorder.net) will trigger the following error message:

swentel\nostr\Relay\CommandResult Object
(
    [success:protected] => 
    [message:protected] => Connection closed: Empty read; connection dead?
    [eventId:protected] => 
)

When actually the event is received successfully by the relays as shown by nosdump request to the relay:

# Using nosdump command in cli 
nosdump --authors 7e7e9c42a91bfef19fa929e5fda1b72e0ebc1a4c1141673e2794234d86addf4e --kinds 9999 wss://nostr-pub.wellorder.net

The event were successfully received by relays but the error message was returned only when $client->close() was called in Relay.php lines 57.

Solution

Instead of using $client->close() in Relay.php lines 57 we can safely replace it with $client->disconnect().

We can follow example in php.net documentation to handle closing of stream_socket_client connection. As shown in the example of php.net documentation for stream_socket_client, they use fclose() to close the socket properly. fclose() is actually used in $client->disconnect() function in connection lib (Lines 328) of phrity/websocket.

Most relays can handle properly client disconnect $client->disconnect() instead of implicit close request $client->close() implementation.

Success Result Example after replacing $client->close() into $client->disconnect():

swentel\nostr\Relay\CommandResult Object
(
    [success:protected] => 1
    [message:protected] => 
    [eventId:protected] => 07abceba124e8b410a0f7c1cfe1a9983c2bee19105d3fb82342ac382b764709e
)

The PR will be sent shortly to solve the issue.

atrifat commented 10 months ago

Related issue #28 .

Sebastix commented 10 months ago

Thanks for digging this out @atrifat ! Just tested the same code on my side I can confirm the same behaviour. Will merge your PR.