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 |
Priority: | Minor | Regression? | |
Status: | Closed/Complete | Components: | 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. |