Skip to content
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

draft/account-registration: should all responses use standard-replies? #542

Open
vanosg opened this issue May 4, 2024 · 2 comments
Open

Comments

@vanosg
Copy link
Contributor

vanosg commented May 4, 2024

This may or may not be an ergo-specific implementation concern, but I don't see anything specifically addressing this in the account-registration spec so figured I'd throw it out to the crowd for opinions.

Per the spec,

Dependencies
This specification uses standard replies framework.

However, on ergo, if you dork up the syntax for account-registration by giving too few arguments (ie, sending only "REGISTER foo", you get a 461 ERR_NEEDMOREPARAMS back. On an Unrealircd 6.1.5 implementation, the same command results in a standard-replies FAIL message being returned

Since standard-replies is still new, I'd be curious on what the intended 'precedence' is for handling errors from account-registration. If the spec dictates standard-replies, should everything be tracked through it? Or does that allow for an existing numeric that fits to still be used instead? Bottom line... should "using the standard-replies framework" be used for every message or not?

As I always favor explicitness in docs whenever possible, if the right answer is to put everything related to account-registration through standard-replies, I would be happy to suggest a note somewhere in that spec (or in standard-replies itself) providing implementation guidance to eliminate any ambiguity.

@ValwareIRC
Copy link
Contributor

Ergo is correct in this case because "Servers which support this capability SHOULD NOT replace standardised error numerics with standard replies, unless the replacement is explicitly described by some other specification.", from the Standard Replies specification. The Account Registration spec doesn't mention a FAIL code for lack of required params, so Ergo is correct in this case.

@slingamn
Copy link
Contributor

slingamn commented May 5, 2024

I agree with @ValwareIRC's interpretation of the specification, but also I think issues like this come up regularly and there's actually not that much at stake here, because the error condition here is a programming error, not a runtime error. It's important to have clear specifications for runtime errors, because clients need to handle them at runtime (in the context of this specification, FAIL REGISTER ACCOUNT_EXISTS is a good example). But there's no way to "handle" 461 ERR_NEEDMOREPARAMS or an equivalent FAIL code --- it's a client code bug and the only solution is to fix the code.

I'm curious what FAIL code Unreal is sending, and whether it's in the registry? But I don't think it's a correctness issue either way, since there's no correct way for client code to make use of the response.

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

No branches or pull requests

3 participants