funbox/smppex

v2 drops handle_parse_error_support.

archseer opened this issue ยท 8 comments

As specified in https://github.com/savonarola/smppex/pull/30, I'd prefer to be able to override the default behavior and handle parse errors by myself (dropping the pdu) instead of automatically stopping my ESME/SMSC.

Our current behavior is we ACK the pdu based on the header, drop it and just keep on going. If we just immediately cut the session, the carrier will just retry the same pdu (and then we stop again -- it's continually killing our session every minute or two).

In v1, we did:

 def handle_parse_error({:unparsed_pdu, raw_pdu, reason}, st) do
    Logger.warn(state: :parse_error, reason: inspect(reason), pdu: inspect(raw_pdu, limit: :infinity))

    command_id = 0x80000000 + raw_pdu.command_id
    code = Errors.code_by_name(:RINVTLVLEN) # TODO: expand to handle more flexibly
    resp = Pdu.new({command_id, code, 0})
    SMPPEX.ESME.reply(self(), raw_pdu, resp)
    {:ok, st}
end

But v2 drops support for that

Ah, that's now handle_unparsed_pdu?

Yes %)

Hmm okay, one more question: How come the handle_send_pdu_result returns st instead of {:ok, st}? It seems inconsistent with all the other callbacks now which take {:ok, st}.

I'll leave some more feedback as we complete our migration ๐Ÿ‘

How come the handle_send_pdu_result returns st

This was made on purpose, because handle_send_pdu_result can be called while a sequence of messages is sent by TransportSession. It would be very confusing to interrupt or extend this sequence with new messages (like other callbacks do), so I chose to receive only new state from handle_send_pdu_result.

Hmm okay, but I'd at least make the response {:ok, st} (without {:ok, [pdus], st}) -- that way it matches all the other responses, won't break backwards compatibility if you decide to support other responses than just updating the state in the future

We had to revert our rollback of v2, both smsc and esme are continually failing with :socket_closed/:tcp_closed.

Edit: Hmm, no idea what the issue is, on v2 SMSC:

** (stop) :socket_closed
Last message: {:tcp_closed, #Port<0.5803>}

=CRASH REPORT==== 9-Nov-2017::08:37:27 ===
crasher:
initial call: Elixir.SMPPEX.TransportSession:init/4
pid: <0.2492.0>
registered_name: []
exception exit: socket_closed
in function  gen_server:handle_common_reply/8 (gen_server.erl, line 726)
ancestors: [<0.2206.0>,<0.2205.0>,ranch_sup,<0.1978.0>]                                                                                                              message_queue_len: 0
messages: []                                                                                                                                                         links: [<0.2206.0>]
dictionary: [{logger_metadata,{true,[{service,smsc}]}}]
trap_exit: true
status: running
heap_size: 610
stack_size: 27
reductions: 474
neighbours:
time=08:37:27.214 level=error    Ranch protocol #PID<0.2492.0> (SMPPEX.TransportSession) of listener #Reference<0.87713570.791150596.82607> terminated
** (exit) :socket_closed
time=08:37:27.214 level=error    GenServer #PID<0.2493.0> terminating
** (stop) :socket_closed
Last message: {:tcp_closed, #Port<0.5802>}

=CRASH REPORT==== 9-Nov-2017::08:37:27 ===
crasher:
initial call: Elixir.SMPPEX.TransportSession:init/4
pid: <0.2493.0>
registered_name: []                                                                                                                                                  exception exit: socket_closed
in function  gen_server:handle_common_reply/8 (gen_server.erl, line 726)                                                                                             ancestors: [<0.2206.0>,<0.2205.0>,ranch_sup,<0.1978.0>]
message_queue_len: 0
messages: []
links: [<0.2206.0>]
dictionary: [{logger_metadata,{true,[{service,smsc}]}}]
trap_exit: true
status: running
heap_size: 610
stack_size: 27
reductions: 478
neighbours:
time=08:37:27.215 level=error    Ranch protocol #PID<0.2493.0> (SMPPEX.TransportSession) of listener #Reference<0.87713570.791150596.82607> terminated
** (exit) :socket_closed

Fixed by using

  def handle_socket_closed(st), do: {:normal, st}

to silence the errors. We're now running v2 in prod, cheers! ๐ŸŽŠ

Great!