I feel like I've had this discussion a few times, but I never remember: if I get an IQ but don't recognize the local name (but do recognize the namespace) should I send back bad-request or feature-not-implemented? conversations.im (ejabberd, I think) sends back bad-request, which makes sense because it doesn't match the schema, but feature-not-implemented seems to overlap and be more specific. Maybe it doesn't matter at all?
debaclehas left
Zash
Sam Whited, your description is basically identical to the text description of feature-not-implemented, so, well, that might be suitable :)
Sam Whited
Seems more or less identical to the description of bad-request too though (it obviously doesn't match the schema for that particular namespace/feature), and that's what ejabberd returns so I was wondering if there's a reason
Zash
Prosody matches most iqs on the (name xmlns) tuple, then says service-unavailable if nothing handles it
jonas’
I send back service-unavailable, because I don’t distinguish "knowing the namespace" vs. "knowing the namespace and the local-name"
Sam Whited
Oh geeze, a third one with overlap. That would be a lot easier actually, I had been planning on sending that back if the namespace wasn't recognized, so maybe I'll just send one error and call it a day.
Sam Whited
Doesn't sound like there's any weird compatibility issues with one vs another anyways, so I might as well start with service-unavailable and then if users complain I can add more fine grained error handling later
samhas left
Sam Whitedhas left
SamWhitedhas left
Zash
I stumbled upon XEP-0045 saying that service-unavailable (and type=wait) means that the room "has reached its maximum number of occupants"
Zash
So, everything is a mess and nothing means anything, do whatever you want
Sam Whited
*sigh* that figures. Thanks for the help
Sam Whited
I can't imagine what the user would do with the knowledge that we do handle the namespace but not the localname anyways, so just having a generic error is probably good enough
Zash
Guess what Prosody says if an external Component isn't connected?
Sam Whited
service-unavailable type='cancel'?
Zash
type=wait
Sam Whited
close.
Zash
Maybe that should be one of the remote-server-somethings
Zash
This led to a client saying "Room is full" when my Biboumi instance wasn't started
Zash
Much fun!
Sam Whited
ejabberd also includes "code='503'"… might just pretend I didn't see that, I really hated the stupid HTTP-like error codes.
Zash
Pretty sure those are deprecated since decades
Sam Whited
yah, if someone complains they can implement it later.
Zash
... Except in MUC!
Sam Whited
I assume Cisco or Google or someone still requires them and someone will eventually complain.
Zash
Can't remember any complaints about Prosody not sending them
Sam Whited
Good to know, thanks
Zash
Sam Whited, was your original question about a client(-library)?
Extarvhas left
Sam Whited
A library, it could be a client or server, just for anything receiving an IQ it doesn't recognize.
Extarvhas joined
Zash
I think it's best to just return service-unavailable as a generic-catch-all
Sam Whited
Yah, I'll do that and type='cancel' so make it clear that they shouldn't retry in this case. Thanks.
Zash
In the case of connected clients, it makes it a bit harder to identify connected resources. Tho you can probably tell from timeing anyways.
Sam Whited
In this case if you were writing a server with this library that would already be handled for you elsewhere, luckily. Can't remember what I send back for that. This fallback is just for user registered handlers that got routed correctly, but now we don't recognize the IQ payload.
sonnyhas left
sonnyhas joined
kikuchiyohas joined
extarvhas joined
debaclehas joined
extarvhas left
lovetoxhas left
kikuchiyohas left
pulkomandyhas left
pulkomandyhas joined
Extarvhas left
lovetoxhas joined
pulkomandyhas left
pulkomandyhas joined
Alexhas left
Alexhas joined
lovetoxhas left
lovetoxhas joined
lovetoxhas left
lovetoxhas joined
Sam Whited
Trying to figure out a good API for registering handlers for different IQ payloads but I just realized my design was assuming 0 or 1 child elements, which is only true for result="true" IQs. I suppose I could match each separate child element in the IQ to a handler, but I bet if you're including multiple things in an IQ they need the context of all the other child elements too.
Sam Whited
And I don't want to decode the entire thing or do any sort of complex query language if I can avoid it; this was supposed to be a bare-bones dead simple way to call events based on elements, fancier stuff can be written by people using the library if they want it, the built in way should just be simple.
Marchas left
Marchas joined
Zash
IQ queries must have exactly one child, responses and errors can have 0 to 2 childs
Kev
get/set = 1, result = [01], error = [12], from memory.
Sam Whited
That's what I had thought, but I couldn't find it in RFC 6120… just did though.
Sam Whited
Thanks, *whew* I was scared I'd have to redo all of that.
Sam Whited
Not sure what I'll do about errors, but I might just ignore the child element from the get or set
Sam Whited
If someone actually wants that payload for some reason they can write their own multiplexer.
Sam Whited
or router or whatever this should be called in XMPP land.
lovetoxhas left
Sam Whited
Actually, no, this doesn't have to care about errors since those are already sent in response to something else and those are automatically returned to the user from the function they used to send the original IQ, not handled as incoming IQs.
Sam Whited
So that all works out nicely and I just need more coffee I guess.
Kev
It's almost always sufficient to route iqs based on name+space of the primary child for get/set, and based on id/from for error/response.
Sam Whited
Yah, that was a much better way to say it, thanks. Doing that.
Zash
Some basic dispatch based on iq type first maybe
Kev
Sorry, yes, I meant type+name+space.
Sam Whited
I'm ignoring type actually (except that get/set and error/response are handled in different places as noted)
Kev
I wouldn't, FWIW, merge get/set.
Sam Whited
yah, I just hand that off to the user; chances are they're going to handle it in the same place with an if statement.
My experience (with implementations that do that) is that it's unhelpful, but YMMV. ✏
Sam Whited
You mean that it's unhelpful to have separate handlers for get/set, or unhelpful to just hand incoming IQs off to the user and let them decide if it's a get or a set?
Kev
It's unhelpful to have to start all handlers with if (get)...
Zash
In Prosody many of the ones that are split in get and set have a bunch of code overlap, tho it feels safer somehow.
Kev
It's not a major wart, but as every handler of which I'm aware needs different behaviour for get and set, it forces every handler anywhere to have the if(get)... scaffolding in it.
Kev
And smart money says if you write enough of those, you'll flip the logic for a fun bug some day.
Kev
Not that I have any experience of that...
Zash
And then one day someone forgets to add that, leading to handling get and set the same way, leading to other things starting to rely on that, leading to a mess.
Kev
So I'd gently encourage splitting of get/set handlers for the sake of your consumers, and then go slink back off to the shadows.
pulkomandyhas left
Sam Whited
Interesting; I'll think about it.
Sam Whited
I'd think they'd share so much logic that you'd want the if get/set instead of having a bigger list of handlers to register, but I defer to experience over me just kind of guessing about it
lovetoxhas joined
Zash
Don't think I've seen /that/ much shared logic
Sam Whited
Actually, yah, on second thought I bet 90% of handlers are just "if get dbLookup else dbExec"
Sam Whited
Maybe it just comes down to whether an if/else or registering two handlers is clearer or requires more boilerplate
Zash
I can imagine that some would share some auth checks
Kev
I think it both comes down to which is clearer, and which is more likely for someone to slip up.
Kev
If you have to register get/set explicitly it's impossible to forget to do so (although you can flip them). If you have to `if` in the call, it's possible to not only flip them, but cause some or all of the code to execute for both when you didn't intend it.
Sam Whited
That makes good sense
Zash
More explicit to have shared/common code factored out in a function