Make UP return 410 after topic has expired

[ShadowJonathan]

This resolves #664;

If a subscription topic;

• Does not have its t.rateVisitor.Stale()
• Does not have any subscribers;
• And the time since last access (including poll and create) has been 16h

...it will start responding with 410s.

This is to make sure that topics eventually will get deleted on servers like mastodon.

There should not be any race condition on first create; Create sets lastAccess to now() the same as a Poll would, through topicsFromIDs, which gets indirectly called from http middleware every time a topic is mentioned, such as here

Unless a client somehow reuses a topic, and sends a topic publish link to the application, which uses it before the client has started its first poll on the topic, this should not create a race condition.

[ShadowJonathan]

Actually, i just double-checked, and this approach will not work, because the Server Manager will prune these topics every minute or so.

Then, when they are created again, their timers are reset.

Even if I were to up this expunge time from 16h to 24h, and/or add an extra test for "if the topic is decaying" (12h), I still think that can be a bit of a broken approach, since it'll basically make a dead topic "removable" only 4h after 12h of retries after it has been referenced by the server again.

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: -0.01% ⚠️

Comparison is base (d7db395) 65.78% compared to head (91a8115) 65.77%.
Report is 3 commits behind head on main.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #842      +/-   ##
==========================================
- Coverage   65.78%   65.77%   -0.01%     
==========================================
  Files          53       53              
  Lines        7680     7682       +2     
==========================================
+ Hits         5052     5053       +1     
  Misses       1788     1788              
- Partials      840      841       +1     

Files Changed	Coverage Δ
server/errors.go	70.00% <ø> (ø)
server/server.go	64.05% <0.00%> (-0.18%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ShadowJonathan]

I'd like to get some feedback on the direction of this change before I'll poke to fix coverage

[karmanyaahm]

since it'll basically make a dead topic "removable" only 4h after 12h of retries after it has been referenced by the server again.

Yeah. You're right, this is just delaying the inevitable by a (relatively small) margin. If this really needs immediate attention, just change the errHTTPInsufficientStorageUnifiedPush to 410 gone.

Like I said before, we'll have to modify the Android app to send NEW_ENDPOINT on reconnect > 12h, and then changing errHTTPInsufficientStorageUnifiedPush to 410 gone will be harmless. I could maybe attempt that over the next week, but no guarantees, it's my first week of college. If you can do it, that would be epic.

[ShadowJonathan]

I'm more concerned how ntfy integrated in other apps would react to something like this, we see this from a mastodon app, most likely fedilab, which means they're using this API for something, what do you think about that?

[binwiederhier]

@karmanyaahm @ShadowJonathan I have no personal stake in this, so I defer to your judgement of whether I should merge this or not. It may be worth thinking through possible race conditions and such. Telling a client to delete the subscription is a serious thing. If we are wrong, a lot of people will start complaining about not receiving notifications anymore.

Also: Apologies for being super late with this. My day job is both exciting and exhausting, so I had very little time for ntfy lately.

[karmanyaahm]

Yeah, let's merge this. Just give me this week to attempt Android re-registration on expiry modifications. It's thanksgiving break so I'm hoping to seriously really get this solved, it's been too long.

[ShadowJonathan]

I'm not sure how to deal with the codecov runs, unfortunately

[karmanyaahm]

Doing this in #979, because it requires some other changes to avoid losing subscriptions permanently.