-
Notifications
You must be signed in to change notification settings - Fork 601
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
remove websocket status code meaning as it is replaced by the CLOSED …
…message.
- Loading branch information
Showing
1 changed file
with
0 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too late but Cancel would have been a better name, since this does not close the socket.
0ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It closes the subscription, like we already have a
CLOSE
message. But I agree maybeCANCEL
would have been better.0ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean?
CLOSED
event from relay to client with indication of specificsubscription_id
is not a substitute to what was removed. I've spent about an hour trying to find which exact code was responsible for that. And probably still gonna use it even though it was removed from NIP-010ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shockingly I agree with @viktorvsk
Gossip client still implements code to detect status code 4000 and to not try reconnecting to relays that send it, and I think that eliminates unnecessary network connections. CLOSED on a subscription isn't enough of a "fuck off" signal for connections, only a "fuck off" about a particular subscription.
0ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly the only use case for the status code 4000 in the wild was nostr.wine telling unauthenticated clients to not try to send
REQ
and then Snort was trying to reconnect a million times because it didn't understand why the connection was being closed, so we reached that agreement and that time.CLOSED
replaces that and now nostr.wine can specify why it isn't serving the request and Snort can behave accordingly.Besides that I don't see why would anyone need a hard "fuck off" -- that would be only for when the relay is being attacked by an evil or buggy client, but in that case the message code won't be respected anyway.
0ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nostr:nevent1qqs2rmj7yqtagc8mehu08zk9e5v2zsd4u0c4063t444xx3evsv0s78spzpmhxue69uhkummnw3ezuamfdejszythwden5te0dehhxarj9ek82tntv5q3samnwvaz7tmjv4kxz7fwdehhxarjv96xjtnrdakszrthwden5te0dehhxtnvdakqz9nhwden5te0wfjkccte9ehx7um5wghxyctwvsq36amnwvaz7tmwdaehgu3dwp6kytnhv4kxcmmjv3jhytnwv46qh5e4d3
0ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. It seemed useful to me, but maybe there is no practical use case.
0ba4589
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, I was just thinking of that specific use case, but if there are others we can bring this back. I think no relay has it implemented anymore (nostr.wine had it, but they changed to
CLOSED
), so it's better to not mention it, I think, unless a justification shows up.