|Summary:||ASTERISK-21903: [patch] Return proper result upon error when running some AGI commands|
|Reporter:||Ariel Wainer (ariw)||Labels:|
|Date Opened:||2013-06-12 09:32:10||Date Closed:||2013-07-18 08:02:02|
|Versions:||22.214.171.124 11.4.0||Frequency of|
|Environment:||Attachments:||( 0) asterisk-21903-return-stream-res_1.8.diff|
( 1) asterisk-21903-return-stream-res_11.diff
|Description:||When playing audio files from an AGI script, if for some reason Asterisk fails to play the file, currently there is no way to tell from the script.
To illustrate, this is the debug transcript from an AGI script trying to play an non-existent file:
<SIP/192.168.200.3-0000007d>AGI Rx << GET OPTION "content/00/3000" ""*0#"" "1000"
[Jun 12 10:48:12] WARNING: file.c:663 ast_openstream_full: File content/00/3000 does not exist in any format
<SIP/192.168.200.3-0000007d>AGI Tx >> 200 result=0 endpos=0
[Jun 12 10:48:12] WARNING: res_agi.c:2003 handle_getoption: Unable to open content/00/3000
Since Asterisk didn't succeed to execute the command, the return code 200 doesn't make sense. I think it should be 404 if the file does not exists or 503 if it doesn't the right permission, to continue with the HTTP analogy.
Even a basic code 500 for anything else than success would be useful.
|Comments:||By: Michael L. Young (elguero) 2013-06-12 12:52:11.244-0500|
Well, I can see where this can cause confusion.
It looks like 200 is to say that the command was run successfully; or rather that the command was found. That part was successful. Now, the fact that the command was unable to complete the task should be made clear by the 'result' code that is part of the result line.
It looks like currently, it always returns 0. In looking at other commands throughout the code, I would say that it should be returning 'result=-1' when there is an error streaming the file.
By: Michael L. Young (elguero) 2013-06-12 12:52:58.795-0500
Give this patch a try [^asterisk-21903-return-stream-res_11.diff] and please report back.
By: Ariel Wainer (ariw) 2013-06-12 13:42:54.140-0500
Michael, tested the patch against the current version (11.4.0), and it works as expected:
<SIP/192.168.200.3-00000001>AGI Rx << GET OPTION "content/static/18/welcome" """" "100"
[Jun 12 15:32:03] WARNING[C-00000001]: file.c:701 ast_openstream_full: File content/static/18/welcome does not exist in any format
<SIP/192.168.200.3-00000001>AGI Tx >> 200 result=-1 endpos=0
[Jun 12 15:32:03] WARNING[C-00000001]: res_agi.c:2069 handle_getoption: Unable to open content/static/18/welcome
Also tried to patch 1.8.10 which is the current packaged version in ubuntu 12.04, but the patch failed to apply:
~/tmp/asterisk-126.96.36.199~dfsg$ sudo patch --verbose -p0<47585_asterisk-21903-return-stream-res_11.diff
Hmm... Looks like a unified diff to me...
The text leading up to this was:
|--- res/res_agi.c (revision 391551)
|+++ res/res_agi.c (working copy)
Patching file res/res_agi.c using Plan A...
Hunk #1 FAILED at 2007.
Hunk #2 FAILED at 2065.
2 out of 2 hunks FAILED -- saving rejects to file res/res_agi.c.rej
By: Ariel Wainer (ariw) 2013-06-12 14:30:29.054-0500
One more observation: It would be nice (although not necessary) to use a different result from the one that comes from hanging up the channel.
It's not really a problem since it's possible to query the channel status.
By: Michael L. Young (elguero) 2013-06-12 14:47:21.862-0500
Yep, I only put a patch up for 11 since that is what you had created the report against.
As far as the result code, are you talking about 'result=-1'? If so, we need to stick to what is being done throughout the code and -1 is what seems to be used to indicate errors, not just hanging up.
There is the channel variable that you can check to determine hangup or not (AGISTATUS).
By: Michael L. Young (elguero) 2013-06-12 14:50:26.788-0500
Here is a patch for 1.8, [^asterisk-21903-return-stream-res_1.8.diff].
By: Ariel Wainer (ariw) 2013-06-12 14:57:37.573-0500
What I meant is that hanging up produces result=-1 as well, but, as you said, you can tell in which situation you are by checking AGISTATUS.
I'll test the 1.8 patch and report back.
By: Ariel Wainer (ariw) 2013-06-13 08:54:11.899-0500
Tested the patch against 1.8, it works as well.