Summary:	ASTERISK-08893: Goto does not proceed to next prio if jump fails
Reporter:	Fredrik Wahl (fw)	Labels:
Date Opened:	2007-02-27 02:57:56.000-0600	Date Closed:	2007-03-05 09:42:44.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/Configuration
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	Behaviour of Goto does not correspond to documentation. Call is terminated if Goto fails. ****** ADDITIONAL INFORMATION ****** According to the docs: *CLI> core show application goto  If the attempt to jump to another location in the dialplan is not successful, then the channel will continue at the next priority of the current extension. Simple test: [default] exten => 123,1,Answer exten => 123,2,Goto(nonext-context,s,1) exten => 123,3,NoOp(Goto failed) exten => i,1,NoOp(Invalid) Result:    -- Executing [123@default:1] Answer("SIP/1041-081bda70", "") in new stack    -- Executing [123@default:2] Goto("SIP/1041-081bda70", "nonext-context|s|1") in new stack    -- Goto (nonext-context,s,1) [Feb 27 09:46:58] WARNING[29319]: pbx.c:2405 __ast_pbx_run: Channel 'SIP/1041-081bda70' sent into invalid extension 's' in context 'nonext-context', but no invalid handler I get a 603 - Declined and the channel is hung up. Shouldn't the execution proceed to prio 3 if the Goto fails or is this desired behaviour? The name of the contexts are read from a database, and I would like to have some error handling if the context for some reason does not exist.
Comments:	By: Brandon Kruse (bkruse) 2007-02-27 20:25:06.000-0600 I am not sure where your WANTING to go, but I believe the syntax is incorrect. By: Fredrik Wahl (fw) 2007-02-28 01:39:50.000-0600 I expected the execution to "continue at the next priority of the current extension" as the description of Goto says if the jump is not successful. In this example, prio 3. If the syntax is incorrect, please enlighten me. By: Serge Vecher (serge-v) 2007-02-28 11:37:18.000-0600 Steve, would you please take a look at this one too? By: Steve Murphy (murf) 2007-02-28 12:30:54.000-0600 The behavior is to abort the call and channel if you can't go where you said to go. I believe that it would result in utter chaos if we changed that behavior. It would be unacceptable, to have a failed goto just proceed to the next extension. Really, the abort is the most merciful thing to do. It forces folks to fix their instructions. So, I think the best thing to do is to fix the docs to give accurate info about what will happen. I have a feeling that changes were made, and the doc strings were overlooked. By: Steve Murphy (murf) 2007-02-28 12:39:32.000-0600 More info: I'm going back to 1.2, where the exact same stuff exists; basically, the target goto info (context, exten, and priority), are written into the channel struct. If that into doesn't correspond to reality, the execution engine (see __ast_pbx_run()) will look for an invalid handler (the 'i' extension), and executate that. If you don't define the 'i' extension, then it looks for the 'h' extension (the hangup extension), and executes that (and hangs up). If it doesn't exist, well, it just hangs up, then. So, I'll try to turn this into somewhat understandable English, and update the docs to reality. By: Steve Murphy (murf) 2007-02-28 13:10:04.000-0600 Actually, looking at the GotoIf: it has wording roughly similar to what Goto says, but it applied specifically to situations where the 'else' or 'if' targets are omitted. It may just be that a copy-paste went wild here, and has been unnoticed these many, many months. By: Steve Murphy (murf) 2007-02-28 13:35:18.000-0600 I added some verbage in 1.2, 1.4, and trunk to pbx.c, for the help-doc strings for Goto, GotoIf, and GotoIfTime, to hopefully make these issues clear. If I misstated something, or got something wrong, or you have a better way to put it, feel free to reopen this issue. The revisions involved are 57118, 57139, and 57140 for 1.2, 1.4, and trunk, respectively. By: Fredrik Wahl (fw) 2007-03-01 02:16:01.000-0600 Thank you for looking into this. I agree that the behaviour can not be changed and an abort is expected if the context is invalid _and_ 'i' and 'h' does not exist in the current context. However c->context is changed in ast_explicit_goto() before the check is done in __ast_pbx_run() if the extensions exist, resulting in a check for 'i' and 'h' in the invalid context and this will always fail. Maybe it is a good idea to check that the context exist in ast_explicit_goto() and not update the struct if it doesn't? 1. Stay in current context and go to ext and prio if they exist. or 2. Stay in current context, invalidate ext and prio, and go to 'i' or 'h' or 3. Stick with it and correct the docs again :) I'm aware that my scenario is kind of different, but I think we all will be happy with correct documentation. By: Steve Murphy (murf) 2007-03-02 11:05:07.000-0600 Yes, you are right. The goto doesn't jump anywhere, it just sets the current context, exten, priority. And that's where the 'h' and 'i' will be looked for when the pbx engine finishes the goto app, just like a PC in a CPU. So, I reworded things again, and hopefully, this will get across all the implications. Reopen if I'm still not on target! Many thanks! fixed in 1.2, 57458      in 1.4, 57473      in trunk, 57476 One other note: it would not be practical to have goto check for the context. At the moment, this could be a very expensive operation, and the pbx engine will check for it immediately after goto is finished anyway. It would just slow things down, and not give a very big benefit. The call is dead no matter if h or i extensions exist or not. By: Fredrik Wahl (fw) 2007-03-05 01:52:19.000-0600 True, but in my opinion it would be even better to stay in the current context and handle the error. It's not a big issue and I can live with it. :) Though, the reason for reopening. You accidentally made a typo on line 356 and 357 (two "and") By: Steve Murphy (murf) 2007-03-05 09:42:44.000-0600 As far as staying in the current context, I actually agree with you, but there's no way to tell you will have problems, without running expensive code to verify that the target of the goto exists. We could do it, but it would seriously slow down the execution engine (in some cases), so I left it out. Having a bad target is disastrous to the execution of the dialplan; whether the 'h' or 'i' extensions exist in the current context is like worrying about the positions of the deck chairs on a sinking Titanic, IMHO... (Somehow this attempt to make users feel better about this particular problem seems doomed to failure!) I removed the extraneous "and"-- it's good you spotted that. I committed this fix to 1.2, 1.4, and trunk via revs 57825 thru 27. Again, feel free to reopen this if further imperfections, disagreements, etc. exist!