Skip to content

harden REST reliability pipeline - #48

Closed
ErrorDoc404 wants to merge 36 commits into
Didas-git:mainfrom
ekretos:improvements/core-reliability
Closed

harden REST reliability pipeline#48
ErrorDoc404 wants to merge 36 commits into
Didas-git:mainfrom
ekretos:improvements/core-reliability

Conversation

@ErrorDoc404

Copy link
Copy Markdown

No description provided.

@brianferri

Copy link
Copy Markdown
Contributor

@ErrorDoc404 What is the purpose of this PR? What warranted the "hardening"?
You haven't described the issue you were trying to solve, and haven't tried to discuss a solution either.

PS. This looks ENTIRELY generated with an LLM. the code quality is very low compared to the rest of the codebase. Complete disregard for code conventions and clear style being followed across the project.

@ErrorDoc404

Copy link
Copy Markdown
Author

You're right that I didn't explain the purpose of the PR clearly enough. The term "hardening" was pretty vague.

What I'm trying to address is reliability in Lilybird's core runtime, mainly around the two areas that interact with external services: REST requests and the Gateway/WebSocket connection.

On the REST side, I wanted a more consistent way of dealing with rate limits and temporary server/network failures. The idea is to retry only where it's safe to do so, while giving callers proper errors for things like rate limits and network failures.

On the Gateway side, the goal is to make reconnects, resumes, heartbeats, and shard connections more reliable, especially when a connection becomes stale or stops responding.

I'm also taking #47 into account since those compiler changes are now part of the direction Lilybird is taking. I'm not trying to pull all of #47 into this PR; I just want the reliability work to fit properly with that architecture instead of adding more coupling to Client.

And regarding the code quality — that's fair. Some of the code in the PR doesn't follow the existing Lilybird conventions closely enough. I should have caught that before opening the PR. I'm going back through the affected files and cleaning them up to match the project's existing style rather than considering the implementation finished just because the functionality works.

I'll also update the PR description so the problem, reasoning, and scope are much clearer.

@brianferri

Copy link
Copy Markdown
Contributor

You're right that I didn't explain the purpose of the PR clearly enough. The term "hardening" was pretty vague.

What I'm trying to address is reliability in Lilybird's core runtime, mainly around the two areas that interact with external services: REST requests and the Gateway/WebSocket connection.

On the REST side, I wanted a more consistent way of dealing with rate limits and temporary server/network failures. The idea is to retry only where it's safe to do so, while giving callers proper errors for things like rate limits and network failures.

On the Gateway side, the goal is to make reconnects, resumes, heartbeats, and shard connections more reliable, especially when a connection becomes stale or stops responding.

I'm also taking #47 into account since those compiler changes are now part of the direction Lilybird is taking. I'm not trying to pull all of #47 into this PR; I just want the reliability work to fit properly with that architecture instead of adding more coupling to Client.

And regarding the code quality — that's fair. Some of the code in the PR doesn't follow the existing Lilybird conventions closely enough. I should have caught that before opening the PR. I'm going back through the affected files and cleaning them up to match the project's existing style rather than considering the implementation finished just because the functionality works.

I'll also update the PR description so the problem, reasoning, and scope are much clearer.

Hi, is this Codex? or... what can I call you? nice to make your acquaintance.
It seems that in attempt to make the library more robust you have inadvertently made it less safe...

The current REST helpers already give you proper errors.
And in this PR you've been removing most of the WS errors that help devs diagnose problems.

You said you wanted to take #47 in to account, but you simply added conflicts against it by "not trying to pull all of" it in.

As to the main point of the PR, in making connections reliable, it's not a concern for the library; its a concern of the user to implement rate limiting. While thoughtful, its out of scope and unwarranted.

I'd love to hear your thoughts on this though, or if you just want to waste a few tokens chatting, thats fine too :)

@Didas-git

Copy link
Copy Markdown
Owner

Rate Limit handling being part of the core is not out of scope just not a priority (see #41), however the state of the PR makes it almost impossible to review.

The changes made to the WebSocket manager make little to no sense to me, they are in no way related to the REST wrappers and the PR seems to remove a lot of its intended errors/warnings.

You have mentioned sharding but I'm afraid whatever program you are using to help write this PR has mislead you, lilybird does not support any form of sharding at its core, and it is intended to give proper errors when users attempt to "force" such features, at least until #42 gets addressed.

My recommendation would be to rebase on #47's branch, and focus only on the rate limiting side of things, other features can be added with extra PRs, but quality must be kept and conventions must be followed, PRs who fail to meet the standards of the repository will not be merged regardless if a human or an AI wrote it.

@Didas-git Didas-git closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants