[Dialogs, Telemetry] The step index is accessed incorrectly when cancelling a waterfall dialog

[v-kydela]

Versions

master branch

Describe the bug

If you cancel a waterfall during a named step (a step not defined by an anonymous function), the telemetry logger will treat it as an unnamed step. I noticed that by reading the code. The bug is not in the other repos though it may have originated by copying code from Python and not adapting it correctly. See this line:

var index = instance.state[state.stepIndex];

state is a local variable that has been assigned the value of instance.state, so instance.state and state are the same thing. It contains a WaterfallDialogState object and not an array, and state.stepIndex is a number, so instance.state[state.stepIndex] will be undefined. The actual step index value can be accessed just with state.stepIndex. In other languages string constants with names like StepIndex are used to access properties of the state object, so I believe that's where the confusion is.

To Reproduce

Steps to reproduce the behavior:

1. Create a waterfall with a telemetry logger and named steps (not defined by anonymous functions)
2. Cancel the waterfall on a named step
3. Observe that the telemetry does not contain the name of the step that the waterfall was cancelled on

Expected behavior

Waterfall cancellation telemetry should include the name of the step that the waterfall that it was cancelled on if that name is available.

Additional context

Fixed in this PR but then rolled back due to scoping: #1578

[bug]

[stevengum]

Thanks for filing this! Can you do the following?

• Demonstrate the incorrect behavior in JS and the difference between C# and JS
  • Telemetry in the Dialogs libraries was first written in C#, and then ported to JS.
• Write a test without adding any new dependencies to botbuilder-dialogs
• If similar tests do not exist in the other languages, please port the test to the other repositories

[v-kydela]

@stevengum - I'm happy to write the tests, but can you explain what it means to demonstrate the incorrect behavior? I've provided repro steps and the failing test will demonstrate the incorrect behavior, so is there something you want beyond that?

[stevengum]

@v-kydela, valid point. Please verify that the behavior between languages is the same by writing a test for the happy path.

What is the error that you mention in step 4 of your reproduction steps? You did not mention a specific error in the original PR or in our in-person discussions.

[v-kydela]

@stevengum - Sorry, I forgot to remove step 4 from the template. It's fixed.