[Home]

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:10Date Closed:2013-07-18 08:02:02
Priority:MinorRegression?
Status:Closed/CompleteComponents:Resources/res_agi
Versions:1.8.22.0 11.4.0 Frequency of
Occurrence
Related
Issues:
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:

{quote}
<SIP/192.168.200.3-0000007d>AGI Rx << GET OPTION "content/00/3000" ""*0#"" "1000"
[Jun 12 10:48:12] WARNING[24554]: 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[24554]: res_agi.c:2003 handle_getoption: Unable to open content/00/3000
{quote}

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.

Best regards,
Ari
Comments:By: Michael L. Young (elguero) 2013-06-12 12:52:11.244-0500

Ariel,

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.

Thanks
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:
{quote}
<SIP/192.168.200.3-00000001>AGI Rx << GET OPTION "content/static/18/welcome" """" "100"
[Jun 12 15:32:03] WARNING[17521][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[17521][C-00000001]: res_agi.c:2069 handle_getoption: Unable to open content/static/18/welcome
{quote}

Also tried to patch 1.8.10 which is the current packaged version in ubuntu 12.04, but the patch failed to apply:
{quote}
~/tmp/asterisk-1.8.10.1~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:
--------------------------
|Index: res/res_agi.c
|===================================================================
|--- 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
{quote}

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.