Asterisk: stasis: set a channel variable on websocket disconnect error
Review Request #4519 - Created March 22, 2015 and submitted
Ashley Sanders | |
branches/13 | |
ASTERISK-24802 | |
Reviewers | |
---|---|
asterisk-dev | |
Asterisk |
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASISSTATUS. The possible values for STASISSTATUS are: SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) And, here it is! https://reviewboard.asterisk.org/r/4520
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises three scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Babs' scenario: tests the situation where a channel is originated under normal conditions and then the channel is hungup. For this case, the test verifies that Stasis correctly assigns SUCCESS to STASISSTATUS. 2) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 3) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
Review request changed
Updated (March 22, 2015, 11:36 p.m.)
-
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASIS_STATUS. The possible values for STASIS_STATUS are: INITIALIZING - Indicates Stasis is starting ACTIVE - The channel is active in Stasis SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.)
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASIS_STATUS. The possible values for STASIS_STATUS are: INITIALIZING - Indicates Stasis is starting ACTIVE - The channel is active in Stasis SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) https://reviewboard.asterisk.org/r/4520
Added a link to the testsuite review.
Posted (March 23, 2015, 4:45 p.m.)
Make sure you document in the CHANGES file that app_stasis now sets a channel variable upon application completion.
-
./branches/13/apps/app_stasis.c (Diff revision 1) -
You'll want to document this new channel variable in the XML documentation at the top of the file. Something like: <para>This application will set the following channel variable upon completion:</para> <variablelist> <variable name="STASISSTATUS"> <para>This indicates the status of the execution of the Stasis application.</para> <value name="SUCCESS" /> <value name="FAILED" /> </variable> </variablelist>
-
./branches/13/res/res_stasis.c (Diff revision 1) -
Since we get a StasisStart event, and the dialplan can't access the variable until the channel is released, I'm not sure we really need to set the STASIS_START variables to INITIALIZING and ACTIVE. With respect to the INITIALIZING value, a Stasis application won't see that value being set on the channel unless they've already subscribed to the channel. The implicit subscription is created as a result of the control_create call, which happens after the variable is set. With respect to the ACTIVE value, this is only set after the StasisStart message is sent - at which point, the application is aware of the channel. When the channel returns from the application, the code in app_stasis will set the value to either "SUCCESS" or "FAILED", which means these values will only have meaning for clients watching the state of the channel through an API. There may be some value in having AMI clients know this state, but I think it may be rather minor - and this will generate at least two Stasis messages per channel entering into the Stasis dialplan application - so I'm not sure it is really worth the overhead.
Posted (March 23, 2015, 5:24 p.m.)
-
./branches/13/apps/app_stasis.c (Diff revision 1) -
Here, I refer to the variable as STASISSTATUS, but below, I refer to it as STASIS_STATUS. This may be a moot point wrt to the source after fixing the issue opened by Matt Jordan, but will impact the test.
Posted (March 24, 2015, 8:31 a.m.)
-
./branches/13/res/res_stasis.c (Diff revision 1) -
+1 to this. I think following the pattern for other applications that do this is good - they just do success/failed.
Just a comment on the code review itself: Please always fill in the "Testing Done" field with the testing you've done.
Posted (March 24, 2015, 1:15 p.m.)
-
./branches/13/apps/app_stasis.c (Diff revision 1) -
Using a parameter value before asserting that it is non-NULL is just wrong. That's just closing the barn door after the barn has been destroyed by the tornado. :)
-
./branches/13/res/res_stasis.c (Diff revision 1) -
Idem.
Review request changed
Updated (March 25, 2015, 3:59 p.m.)
-
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises three scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Babs' scenario: tests a nominal path of Stasis to verify the 'ACTIVE' state is correctly applied. For this test, a call is originated under normal conditions and then the system is polled for the value of STASIS_STATUS before the channel is hung up. 2) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 3) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
-
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASIS_STATUS. The possible values for STASIS_STATUS are: INITIALIZING - Indicates Stasis is starting ACTIVE - The channel is active in Stasis SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) https://reviewboard.asterisk.org/r/4520
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASISSTATUS. The possible values for STASIS_STATUS are: INITIALIZING - Indicates Stasis is starting ACTIVE - The channel is active in Stasis SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) https://reviewboard.asterisk.org/r/4520
Updated the description and testing done fields to depict: A) The name of the channel variable is: STASISSTATUS, not STASIS_STATUS B) The testing done field now contains a link and the blurb from the Testsuite test, for clarity.
Review request changed
Updated (March 27, 2015, 9:54 a.m.)
-
- added Diff r2 - Show changes
Addresses review feedback from mjordan, jcolp, and rmudgett.
Review request changed
Updated (March 27, 2015, 9:59 a.m.)
-
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASISSTATUS. The possible values for STASIS_STATUS are: INITIALIZING - Indicates Stasis is starting ACTIVE - The channel is active in Stasis SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) https://reviewboard.asterisk.org/r/4520
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASISSTATUS. The possible values for STASIS_STATUS are: SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) And, here it is! https://reviewboard.asterisk.org/r/4520
Updated the description to depict the revised values of STASISSTATUS.
Review request changed
Updated (March 27, 2015, 10:01 a.m.)
-
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises three scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Babs' scenario: tests a nominal path of Stasis to verify the 'ACTIVE' state is correctly applied. For this test, a call is originated under normal conditions and then the system is polled for the value of STASIS_STATUS before the channel is hung up. 2) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 3) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises three scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Babs' scenario: tests a nominal path of Stasis to verify the 'ACTIVE' state is correctly applied. For this test, a call is originated under normal conditions and then the system is polled for the value of STASISSTATUS before the channel is hung up. 2) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 3) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
-
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASISSTATUS. The possible values for STASIS_STATUS are: SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) And, here it is! https://reviewboard.asterisk.org/r/4520
When an error occurs while writing to a web socket, the web socket is disconnected and the event is logged. A side-effect of this, however, is that any application on the other side waiting for a response from Stasis is left hanging indefinitely (as there is no mechanism presently available for notifying interested parties about web socket error states in Stasis). To remedy this scenario, this patch introduces a new channel variable: STASISSTATUS. The possible values for STASISSTATUS are: SUCCESS - The channel has exited Stasis without any failures FAILED - Something caused Stasis to croak. Some (not all) possible reasons for this: - The app registry is not instantiated; - The app requested is not registered; - The app requested is not active; - Stasis couldn't send a start message ***Note*** This is just the patch to the Asterisk source. The testsuite review is coming soon to a reviewboard near you (well, this reviewboard.) And, here it is! https://reviewboard.asterisk.org/r/4520
Changed the references to STASIS_STATUS to STASISSTATUS.
Review request changed
Updated (March 27, 2015, 10:59 a.m.)
-
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises three scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Babs' scenario: tests a nominal path of Stasis to verify the 'ACTIVE' state is correctly applied. For this test, a call is originated under normal conditions and then the system is polled for the value of STASISSTATUS before the channel is hung up. 2) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 3) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises two scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 2) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
Updated the 'Testing Done' field to reflect the modifications to the testsuite issue's description change.
Review request changed
Updated (March 28, 2015, 12:38 a.m.)
-
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises two scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 2) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
The testing done for this issue can be found at: https://reviewboard.asterisk.org/r/4520/ The test in Testsuite exercises three scenarios to elicit updates to the STASISSTATUS channel variable: 1) The 'Babs' scenario: tests the situation where a channel is originated under normal conditions and then the channel is hungup. For this case, the test verifies that Stasis correctly assigns SUCCESS to STASISSTATUS. 2) The 'Bugs' scenario: tests the situation where a call is originated requesting an app that was never registered in Stasis to verify the 'FAILED' state is correctly applied. 3) The 'Buster' scenario: tests the situation where an app that was registered in Stasis when call A was originated (and while call A is still active) but is no longer registered when call B is originated. Determines if the 'FAILED' state is correctly applied.
Updated the 'testing done' field to depict the re-added 'Babs' scenario.
-
./branches/13/apps/app_stasis.c (Diff revision 2) -
Nitpick: "A failure occurred when executing the Stasis application."
Otherwise good.
Review request changed
Updated (March 31, 2015, 4:54 p.m.)
-
- added Diff r3 - Show changes
Fixed the docstring according to Joshua Colp's suggestion.
Review request changed
Updated (March 31, 2015, 6:23 p.m.)
- changed from pending to submitted
Committed in revision 433861
Other reviews