Alex, I didn't expect the patch to be accepted, it was more of a "conversation starter". Anyway, perhaps it could prove useful for some people or at least as a guide (even about how NOT to do things) ;)
Passing the error code on ACK was more about consistency than anything else. The protocol, as Juan mentioned, is intended to allow multi-protocol load balancing. It's not really needed if you stay on a single protocol, otherwise you'd get different error codes for the same error depending on the protocol you're into. Regards, Alex On Thu, Oct 25, 2012 at 12:01 PM, Alexander Malysh <[email protected]>wrote: > Hi Alex, > > this patch is no go, because you put SMPP specific handling into abstract > bb_smscconn. > > As to the discussion itself. We use error codes in our apps as well and we > were never in the > situation that we have to know protocol from the meta-data or message > itself. We have mapping > of SMSC name and which protocol it uses BUT we this only as exception. > > We can put protocol into NACK with error code but it make no sense for me > to put protocol into ACK as well as put > error code into ACK. ACK means accepted and I don't see any sense to > forward protocol specific ack error code > to the application layer. Am I missing something then please clarify why > do you need numeric value for ACK? > > If we decide to put protocol into NACK message then we should implement > additional callback function (get_protocol) or > something like this and then write it out to application layer but not as > you did it with mixing SMSC abstract layer and implementation > specific things. > > Alex > > Am 25.10.2012 um 00:56 schrieb Alejandro Guerrieri < > [email protected]>: > > Hi, > > Attached is a patch that allows passing the command_status on SMPP as > meta-data, on the smpp_error_code parameter, so on the logs (and dlr-url) > you'd get something like: > > *[META:?smpp?smpp_error_code=72&]* > * > * > This would be command status value (which is 0 for accepted messages and a > positive integer for any other outcome). > > This is based on some code Donald Jackson wrote for us time ago, and while > this might prove useful for some people, after speaking with Stipe he > mentioned that we can achieve similar results by parsing the %A parameter > on the dlr-url. In that case, the possible outcomes right now are: > > *ACK/* > *NACK/0xXXXXXXXX/Error text* > > In this case, for an accepted message, no numeric code is given. > For a rejected message, 0xXXXXXXXX contains the hex value of the > command_status > > After discussing for a little bit, Stipe pretty much talked us out of the > idea of our patch because it's non-generic (only works for SMPP). > > However, %A is far from perfect as well: > > 1. It's inconsistent. No numeric value is provided on ACK's. > 2. You cannot tell the protocol, so if you're load-balancing binds with > different protocols you'd get different error codes with no way to > correlate them by protocol. > 3. It's not documented either. > > After a very healthy debate, we thought of the following approach, keeping > the %A method: > > *ACK/SMPP:0x00000000/OK* > *NACK/SMPP:0x00000048/Invalid Source address TON* > > This slightly breaks compatibility if you were already parsing the error > code on the NACK's or doing an exact match on "ACK/" but imho is not a big > deal. > > Thoughts? Ideas? Is it something other people would be interested into? > > Regards, > > Alex > <kannel-command-status.patch> > > >
