New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prepare v4.1.3 release #25532
Prepare v4.1.3 release #25532
Conversation
if (isBinary) { | ||
log.warn('socket', 'Received binary data, closing connection'); | ||
ws.close(1003, 'The mastodon streaming server does not support binary messages'); | ||
return; |
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.
@ClearlyClaire I think due to IceCubes we may need to revert these lines.. IceCubes doesn't feature an exponential backoff, reconnect jitter or max reconnection attempts on the websocket, so users using IceCubes against 4.1.3 end up just hammering the server with connection attempts. Obviously this isn't good client behaviour and we need to work with the IceCubes developer to help them better interact with websockets, I've started that communication.
I'll do up a 4.1.3 based patch to revert to previous behaviour, but I think we should keep as-is for 4.2.0.. it's so far just one misbehaving client, according to @shleeable
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 agree it's safer to revert those changes. I think we might do that as well on main
for the time being.
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.
IceCubes has just fixed this issue on their side.
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.
As IceCubes has released a fix, I think we can proceed with releasing this.
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'm not sure we should ship a behavior change like this in a patch release, it's known to have broken one client, and I'm not sure it fixes anything worth fixing in the short term.
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.
The streaming API documentation (which is only findable via google search) doesn't actually specify the framing for websocket messages, it just says "JSON encoded", which almost always implies JSON sent as a string
We could ask @andypiper, but I'm pretty sure there's no other implementations of the streaming API.
Though, if you'd prefer to save this change for 4.2.0, then I can write up a patch for 4.1.3 to handle this differently.
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.
Noting that we need to make the streaming API docs more findable!
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.
@andypiper turns out it is linked, but it's nested under the "timelines" section, which wasn't where I thought it'd be.
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.
@ThisIsMissEm would a flat list be better? the reason the sidebar is nested is because it was helpful to group api routes by conceptual area rather than completely unsorted or alphabetical sorting. i.e., streaming is something you'd want if you were building an app that handled timelines. tangentially, i have theme changes to collapse the main menu to only the current section, and also to add a search bar, i'm just not sure if or when they would be merged...
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 think it's because it's under REST API and the API Methods, so I just wasn't looking for streaming information there.. streaming can be related to timelines, but also notifications and profile's posts.
0ca44f4
to
4cf9e0b
Compare
4cf9e0b
to
cb45d40
Compare
Co-authored-by: Claire <claire.github-309c@sitedethib.com>
cb45d40
to
e36a9cf
Compare
e36a9cf
to
f5b328b
Compare
Added
LOCAL_DOMAIN@LOCAL_DOMAIN
(ClearlyClaire)Changed
Removed
X-Frame-Options: ALLOWALL
(ClearlyClaire)Fixed
/api/v2/admin/accounts
(danielmbrasil)S3_ALIAS_HOST
includes a path component (ClearlyClaire)tootctl accounts approve --number N
not aproving N earliest registrations (danielmbrasil)