Skip to content
This repository was archived by the owner on Jan 24, 2026. It is now read-only.

Remove annoying mute error#53

Closed
staab wants to merge 1 commit into
fiatjaf:masterfrom
coracle-social:no-mute-warn
Closed

Remove annoying mute error#53
staab wants to merge 1 commit into
fiatjaf:masterfrom
coracle-social:no-mute-warn

Conversation

@staab

@staab staab commented Jun 6, 2025

Copy link
Copy Markdown

This broke flotilla, and now it's breaking alby go. Maybe send a notice instead?

@frnandu

frnandu commented Jun 6, 2025

Copy link
Copy Markdown

+1, we're using khatru with rabbitmq for horizontal scaling, so even if one instance of the relay has no listeners locally it will broadcast to rabbitmq from which other instances consume the event and they might have listeners.

@fiatjaf

fiatjaf commented Jun 7, 2025

Copy link
Copy Markdown
Owner

@staab how is it breaking Alby Go?

@fiatjaf fiatjaf closed this in 034902c Jun 7, 2025
@fiatjaf

fiatjaf commented Jun 7, 2025

Copy link
Copy Markdown
Owner

@frnandu how is the event being sent to rabbitmq? Now if you have a hook for handling ephemeral events it won't trigger the mute anymore.

If you just hate your users and don't want to let them know that they're mute you can just implement an empty OnEphemeralEvent hook.

@staab

staab commented Jun 7, 2025

Copy link
Copy Markdown
Author

This is all I know: nostr:nevent1qvzqqqqqqypzpggzvz325tcf9kz79s9c9627430ccc82r8rgujycwxd43n92y037qqsvptfarkk7w4xy9hp5yzl0zfvtuvdjzmghlctxmnsr24rh5nlaj7q0e2khv

@fiatjaf

fiatjaf commented Jun 7, 2025

Copy link
Copy Markdown
Owner

Why do you say it's "breaking" anything? Looks like it's a feature and working as intended.

Now the app or the user can know what happened in a case where otherwise it would just fail silently.

The only thing that could be interpreted as "breaking" here is if he is sending the requests to many relays and it would have worked through another relay but now the entire process fails because a Khatru relay returned an ok=false.

But this is just a bug in the NWC client, it would also fail if the relay was offline? Like I'm sending the request to 4 relays and one of them would have worked, but because 3 are offline everything fails.

@staab

staab commented Jun 9, 2025

Copy link
Copy Markdown
Author

Not all ephemeral events need listeners. This can be the case if if the relay itself is a listener, for example Flotilla uses an error to indicate a rejection of a relay access request (nostr-protocol/nips#1079). But it can also the be the case that you might broadcast an ephemeral event and not care if anyone is listening — but want to know if it's rejected by the relay. The two things are different types of errors (more like an error vs a warning) and should probably be handled differently.

@fiatjaf

fiatjaf commented Jun 9, 2025

Copy link
Copy Markdown
Owner

I think the case when the relay itself is the listener is covered now since 034902c.

The second case is weird, but I guess you want that for probing reasons. It doesn't feel like the best approach though. And the benefits of having the relay give you an unequivocal error when your message didn't reach anyone are pretty significant I think.

Maybe if a relay wants to just do nothing but also not error they can implement the ephemeral hook handler.

Hmm, maybe that hook handler should be allowed to return an error too...

@staab

staab commented Jun 9, 2025

Copy link
Copy Markdown
Author

Probing yes, but I'm also thinking of some kind of heartbeat event or live status thing (I have no concrete use cases in mind though), where ephemeral might be the right range, but no one might care about events the user publishes.

Also re: OnEphemeralEvent, I have my logic in RejectEvent, since the event should be rejected if the invite code is invalid.

@fiatjaf

fiatjaf commented Jun 9, 2025

Copy link
Copy Markdown
Owner

I think khatru might not be generic enough.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants