Sam WhitedI 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?
ZashSam Whited, your description is basically identical to the text description of feature-not-implemented, so, well, that might be suitable :)
Sam WhitedSeems 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
ZashProsody 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 WhitedOh 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 WhitedDoesn'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
Sam Whitedhas left
ZashI stumbled upon XEP-0045 saying that service-unavailable (and type=wait) means that the room "has reached its maximum number of occupants"
ZashSo, everything is a mess and nothing means anything, do whatever you want
Sam Whited*sigh* that figures. Thanks for the help
Sam WhitedI 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
ZashGuess what Prosody says if an external Component isn't connected?
Sam Whitedservice-unavailable type='cancel'?
ZashMaybe that should be one of the remote-server-somethings
ZashThis led to a client saying "Room is full" when my Biboumi instance wasn't started
Sam Whitedejabberd also includes "code='503'"… might just pretend I didn't see that, I really hated the stupid HTTP-like error codes.
ZashPretty sure those are deprecated since decades
Sam Whitedyah, if someone complains they can implement it later.
Zash... Except in MUC!
Sam WhitedI assume Cisco or Google or someone still requires them and someone will eventually complain.
ZashCan't remember any complaints about Prosody not sending them
Sam WhitedGood to know, thanks
ZashSam Whited, was your original question about a client(-library)?
Sam WhitedA library, it could be a client or server, just for anything receiving an IQ it doesn't recognize.
ZashI think it's best to just return service-unavailable as a generic-catch-all
Sam WhitedYah, I'll do that and type='cancel' so make it clear that they shouldn't retry in this case. Thanks.
ZashIn the case of connected clients, it makes it a bit harder to identify connected resources. Tho you can probably tell from timeing anyways.
Sam WhitedIn 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.
Sam WhitedTrying 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 WhitedAnd 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.
ZashIQ queries must have exactly one child, responses and errors can have 0 to 2 childs
Kevget/set = 1, result = , error = , from memory.
Sam WhitedThat's what I had thought, but I couldn't find it in RFC 6120… just did though.
Sam WhitedThanks, *whew* I was scared I'd have to redo all of that.
Sam WhitedNot sure what I'll do about errors, but I might just ignore the child element from the get or set
Sam WhitedIf someone actually wants that payload for some reason they can write their own multiplexer.
Sam Whitedor router or whatever this should be called in XMPP land.
Sam WhitedActually, 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 WhitedSo that all works out nicely and I just need more coffee I guess.
KevIt'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 WhitedYah, that was a much better way to say it, thanks. Doing that.
ZashSome basic dispatch based on iq type first maybe
KevSorry, yes, I meant type+name+space.
Sam WhitedI'm ignoring type actually (except that get/set and error/response are handled in different places as noted)
KevI wouldn't, FWIW, merge get/set.
Sam Whitedyah, I just hand that off to the user; chances are they're going to handle it in the same place with an if statement.
KevMy experience (with implementations that do that) is that it's unhelpful, but YMMV.
Sam WhitedYou 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?
KevIt's unhelpful to have to start all handlers with if (get)...
ZashIn Prosody many of the ones that are split in get and set have a bunch of code overlap, tho it feels safer somehow.
KevIt'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.
KevAnd smart money says if you write enough of those, you'll flip the logic for a fun bug some day.
KevNot that I have any experience of that...
ZashAnd 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.
KevSo I'd gently encourage splitting of get/set handlers for the sake of your consumers, and then go slink back off to the shadows.
Sam WhitedInteresting; I'll think about it.
Sam WhitedI'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
ZashDon't think I've seen /that/ much shared logic
Sam WhitedActually, yah, on second thought I bet 90% of handlers are just "if get dbLookup else dbExec"
Sam WhitedMaybe it just comes down to whether an if/else or registering two handlers is clearer or requires more boilerplate
ZashI can imagine that some would share some auth checks
KevI think it both comes down to which is clearer, and which is more likely for someone to slip up.
KevIf 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 WhitedThat makes good sense
ZashMore explicit to have shared/common code factored out in a function