Re: [PATCH] SMPP command_status as meta-data

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>
>
>
>