verifies $response type

[bits4breakfast]

ensures that response from ClientImpl::findResponse() is PAMI\Message\Response or false before calling PAMI\Message\Response::addEvent()

[marcelog]

Hi :) Could you give a use case? When/Why is this happening? What event(s) and asterisk version(s)? Could you paste the raw events (and actions) that cause this?

[bits4breakfast]

Hi - Honestly I would need some time to collect more details, but I've just noticed that an application I've built that uses your library throws this error by time to time

E_ERROR: Call to a member function addEvent() on a non-object

this is a portion of the stack trace

…ses/20150331120320/vendor/marcelog/pami/src/mg/PAMI/Client/Impl/ClientImpl.php (285)
…ses/20150331120320/vendor/marcelog/pami/src/mg/PAMI/Client/Impl/ClientImpl.php (410)
…150331120320/src/Facile/Ws/BunnyBundle/Controller/Utils/AsteriskController.php (110)

[marcelog]

That's ok, take your time. I'm interested in knowing if this is just a bug in pami, or a strange asterisk behavior, etc.

[bits4breakfast]

is there anything in particular that I can check for you? I'm not very familiar with your library and asterisk protocol, I've just noticed few errors here and there and I decided to contribute back with this fix

[marcelog]

Exactly what I mentioned earlier, which basically reduces to how to reproduce this behavior, including the asterisk version, action(s) sent (if any), and events/responses that produce this behavior.

[StrikeForceZero]

I will comment on this and say I encountered the same issue; reproducible sending QueuesAction on asterisk 11.

This PR does address the exception as the function findResponse does return false when the response does not have an ActionId..

The comments above do say:

// broken ami.. sending a response with events without Event and ActionId

so I'm not sure what the proper way to handle this scenario

[marcelog]

Hi @StrikeForceZero,

Could you confirm this is the issue you're experiencing? https://issues.asterisk.org/jira/browse/ASTERISK-23821

[StrikeForceZero]

ah... yup that would be it

Testing Queues Action

Action: Queues
ActionID: 123456789

default has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime, 0s talktime), W:0, C:0, A:0, SL:0.0% within 0s
No Members
No Callers

Testing Reload Action

Action: Reload
ActionID: 12345678

Response: Success
ActionID: 12345678
Message: Module Reloaded

Testing QueueStatus

Action: QueueStatus
ActionID: 123456

Response: Success
ActionID: 123456
Message: Queue status will follow

Event: QueueParams
Queue: default
Max: 0
Strategy: ringall
Calls: 0
Holdtime: 0
TalkTime: 0
Completed: 0
Abandoned: 0
ServiceLevel: 0
ServicelevelPerf: 0.0
Weight: 0
ActionID: 123456

Event: QueueStatusComplete
ActionID: 123456```

[marcelog]

Thanks for the confirmation.. Unfortunately it seems we should wait until Digium fixes it.

Going to close this one, but please feel free to come back and report any progress!

[StrikeForceZero]

In the meantime maybe the best route would be to tweak @bits4breakfast PR to throw an Exception that the user can optionally catch? EmptyActionIDException($event)

That would allow the user to determine what to do instead of being fatal.
And maybe also note the bug in a comment for Asterisk 11 Queues Action? (might prevent future issues)

@marcelog your thoughts?

[marcelog]

mmm This line here https://github.com/marcelog/PAMI/blob/master/src/mg/PAMI/Client/Impl/ClientImpl.php#L283 should "automagically" add the action id and the response should exists.. Could you do a few tests? For example dumping $event and also $message->getActionId(); inside findResponse.

[marcelog]

By the way, note how the response does not match the AMI protocol:

default has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime, 0s talktime), W:0, C:0, A:0, SL:0.0% within 0s
No Members
No Callers

This is a similar issue to #88 so I don't see a right way of handling it, the response is just not respecting the AMI format for the event and response.

QueueStatus should be the way to go.

[StrikeForceZero]

dumping $event and $message->getActionId(); give the following when calling Queues Action:

action id: null
event: UknownEvent

Event: ResponseEvent\r\n
ActionId: 1444139694.8284\r\n
200 has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime, 0s talktime), W:1, C:0, A:0, SL:0.0% within 180s\r\n
Members: \r\n
Some Person (Local/201@from-queue/n from hint:201@ext-local) (ringinuse enabled) (dynamic) (Unavailable) has taken no calls yet\r\n
No Callers

action id: 1444139694.8284
event: UknownEvent

Event: ResponseEvent\r\n
ActionId: 1444139694.8284\r\n
300 has 0 calls (max unlimited) in 'ringall' strategy (0s holdtime, 0s talktime), W:1, C:0, A:0, SL:0.0% within 180s\r\n
Members: \r\n
Some Person (Local/301@from-queue/n from hint:301@ext-local) (ringinuse enabled) (dynamic) (Unavailable) has taken no calls yet\r\n
No Callers

[All the queues...]

ClientException: Read Timeout

So yes, it appears its adding the ActionID

[StrikeForceZero]

you are right as far as QueueStatus being the way to go. Maybe it wouldn't be bad to deprecate Queue Action and suggest QueueStatus for more consistent/reliable behavior across different versions of Asterisk.