search

binwiederhier / ntfy

Send push notifications to your phone or desktop using PUT/POST
https://ntfy.sh
Apache License 2.0
17.23k stars 662 forks source link

UnifiedPush: Respond with 404/409/... to Mastodon/etc. instead of 507 based on User-Agent #664

Open binwiederhier opened 1 year ago

binwiederhier commented 1 year ago

Similar to the Matrix Push Gateway change, we should respond to Mastodon/etc. in a way that will make them delete the push subscription.

Mastodon: https://github.com/mastodon/mastodon/blob/730bb3e211a84a2f30e3e2bbeae3f77149824a68/app/workers/web/push_notification_worker.rb#L35-L46

Common-Proxies: https://github.com/UnifiedPush/common-proxies/blob/main/gateway/matrix.go#L63

/cc FYI @karmanyaahm @p1gp1g

p1gp1g commented 1 year ago

Returning 404 for all non existant UnifiedPush requests (where up=1) will follow UnifiedPush spec and Webpush spec and probably avoid many issues :)

RFC 8030 (Webpush)

A push service MAY expire a subscription at any time. If there are outstanding requests to an expired push message subscription resource (Section 6) from a user agent or to an expired receipt subscription resource (Section 6.3) from an application server, this MUST be signaled by returning a 404 (Not Found) status code.

UnifiedPush

404

The endpoint does not exist and SHOULD NOT be used anymore. Implementing this is OPTIONAL for both the application server and push server.

p1gp1g commented 1 year ago

From the discussion in chat: Maybe returning 404 after some time (12h ?) for the same reasons we chose to do so for matrix ?

binwiederhier commented 1 year ago

@karmanyaahm @p1gp1g I am inclined to close this, since nobody has complained. The 5xx rates are still high, but I have just learned to live with it ... Thoughts?

karmanyaahm commented 1 year ago

If no one's having noticable problems, it ought to be fine. Still kinda weird it hasn't normalized down though :/

Lastorder-DC commented 12 months ago

EDITED by @binwiederhier: Removed image containing lots of 507 errors to UP topics. The UP topics are secrets, so I removed the picture.

It keeps happening and increasing.

binwiederhier commented 12 months ago

@Lastorder-DC please censor the picture. Unified Push topics are secrets.

Edit: I took the liberty of removing the image for you.

binwiederhier commented 12 months ago

This is happening because your server is pushing messages to topics that do not have any subscribers. They are dead topics.

The alternative to returning 507 would be to let them through, but then you'd get rate limited and banned within a matter of minutes. So you can either live with the errors, or manually purge the topics from Mastodon.

I am also ok with pull requests to handle it specifically for Mastodon if there are suggestions.

Lastorder-DC commented 12 months ago

@binwiederhier Forgot that. Sorry! (Actually I have sensored them but accidently choose non-sensored version...)

ShadowJonathan commented 11 months ago

We're seeing a buildup of push messages to ntfy.sh, presumably to UP. Returning a 4XX code so that the queue is deleted would help a lot, since we're continually getting IP banned by the sheer volume of requests backed up.

Also relevant: https://github.com/mastodon/mastodon/issues/26078

ShadowJonathan commented 10 months ago

I've talked with @karmanyaahm about this issue, and the consensus seems to be that, 12h after deletion, nfty should be returning 410s for a given subscription.

ShadowJonathan commented 10 months ago

Actually, the .Stale() function will give a better approximation of when to expire.