jovo-framework icon indicating copy to clipboard operation
jovo-framework copied to clipboard

`Unhandled` and `@PrioritizedOverUnhandled` handlers don't clean up the state stack correctly

Open jtfell opened this issue 3 years ago • 10 comments

I'm submitting a...

  • [x] Bug report
  • [ ] Feature request
  • [ ] Documentation issue or request
  • [ ] Other... Please describe:

Expected Behavior

I'll use @PrioritizedOverUnhandled as the example here, but I believe this issue applies more generally when delegated components aren't explicitly resolved.

If I have a handler with the @PrioritizedOverUnhandled() decorator for a specific intent and its called, the state stack is left in a way that breaks the $resolve behaviour.

See the reproduction here.

Current Behavior

I did this interaction through the debugger:

image

And saw this in the state stack. It did not find the handler in the GlobalComponent via the resolve call in LoveHatePizzaComponent

 >>>>> Request - 2022-02-01T00:53:32.267Z
{
  "version": "4.0",
  "platform": "jovo-debugger",
  "id": "d79b0a70-3c4d-4efd-947e-6cae966b51cc",
  "timestamp": "2022-02-01T00:53:31.979Z",
  "timeZone": "Australia/Brisbane",
  "locale": "en",
  "input": {
    "intent": "NoGoodIntent",
    "type": "INTENT"
  },
  "context": {
    "device": {
      "id": "b019fe87-cae7-4f79-a5cb-fb6e38e0fe29",
      "capabilities": [
        "SCREEN",
        "AUDIO"
      ]
    },
    "session": {
      "id": "0bf34a99-531a-4969-b694-e26eaaf6849f",
      "data": {},
      "state": [
        {
          "component": "GlobalComponent"
        },
        {
          "resolve": {
            "fail": "Fail"
          },
          "component": "LoveHatePizzaComponent"
        },
        {
          "resolve": {
            "yes": "lovesPizza"
          },
          "config": {
            "outputOpts": {
              "message": "Do you like pizza?"
            }
          },
          "component": "LoveHatePizzaComponent.YesNoComponent",
          "data": {
            "numberOfTries": 0
          }
        }
      ],
      "isNew": false,
      "updatedAt": "2022-02-01T00:53:30.065Z",
      "new": false
    },
    "user": {
      "id": "81390558-0b21-4d30-b0b5-e8928bc79174",
      "data": {}
    }
  }
}

 <<<<< Response - 2022-02-01T00:53:32.275Z  ✔️ 8ms
{
  "version": "4.0.0",
  "platform": "jovo-debugger",
  "output": [],
  "context": {
    "request": {},
    "session": {
      "end": false,
      "data": {},
      "id": "0bf34a99-531a-4969-b694-e26eaaf6849f",
      "state": [
        {
          "component": "GlobalComponent"
        },
        {
          "resolve": {
            "fail": "Fail"
          },
          "component": "LoveHatePizzaComponent"
        },
        {
          "component": "LoveHatePizzaComponent"
        }
      ]
    },
    "user": {
      "data": {}
    }
  }
}

It seems like this logic doesn't get called when routed in this way, and there is that extra

{
  "component": "LoveHatePizzaComponent"
}

Error log

If you have an error log, please paste it here.

Your Environment

  • Jovo Framework version used: 4.0.2
  • Operating System: MacOS

jtfell avatar Feb 01 '22 02:02 jtfell

Thank you for flagging this @jtfell!

The $state stack not getting cleaned up correctly was also a concern of mine when we were designing it. I'll take a closer look and think about solutions for this.

jankoenig avatar Feb 10 '22 09:02 jankoenig

@jankoenig @aswetlow Is component redirect ready for production?

Is there a function to clear the state stack?

If in GlobalComponent I redirect to LoveHatePizzaComponent:

  PizzaIntent() {
    return this.$redirect(LoveHatePizzaComponent);
  }

And I also add a GlobalComponent.START:

  START() {
    this.$send('What do you want to do now?');
  }

Then in LoveHatePizzaComponent after I answer YES then I redirect back to GlobalComponent. I should no longer be in LoveHatePizzaComponent state:

  @Intents(['YesIntent'])
  lovesPizza() {
    this.$send({ message: 'Yes! I love pizza, too.', listen: true});
    return this.$redirect(GlobalComponent);
  }

Here is the response:

{
   "version": "4.0.0",
   "platform": "jovo-debugger",
   "output": [
     {
       "message": "Yes! I love pizza, too."
       "listen": true
     },
     {
       "message": "What do you want to do now?"
     }
   ],
   "context": {
     "request": {},
     "session": {
       "end": false,
       "data": {},
       "id": "ade99cc9-1930-4038-921b-16d5890e2231",
       "state": [
         {
           "component": "LoveHatePizzaComponent"
         }
       ]
     },
     "user": {
       "data": {}
     }
   }
 }

rmtuckerphx avatar Apr 26 '22 02:04 rmtuckerphx

You can clear the state stack with this.$state = [].

That example looks interesting, we'll take a look. The $redirect should replace the "old" component with the new one. Maybe the problem here is that it's being redirected to GlobalComponent (global components don't get added to the state stack), that could be a potential bug on our side.

I tend to think that, to solve this stacking problem, we should not only replace the "old" component with the new one when using $redirect, but clear the full $state. What do you think @aswetlow?

jankoenig avatar Apr 26 '22 08:04 jankoenig

I'm having this issue as well. I have all the packages updated to latest version.

This is happening to me only in Google Assistant.

Jovo Debugger and Alexa do clear the state correctly.

My flow:

  1. GlobalComponent.LAUNCH redirects to ComponentA---> All platforms have ComponentA in state.
  2. User says global intent and Jovo finds it in GlobalComponent. (It is annotated with @PrioritizedOverUnhandled and it just redirects to ComponentB)
  3. The state attribute of the following response is wrong (only in Google Assistant). It's ComponentA when it should be ComponentB.

Note: neither componentA nor componentB are global.

jrglg avatar Jun 06 '22 16:06 jrglg

Hey @jrglg Could you provide a code example to be able to reproduce it?

aswetlow avatar Jun 07 '22 14:06 aswetlow

Hi @aswetlow

here you have it.

test.zip

You have to create a blank project named in gactions named 'test-state-redirect'.

Then:

  1. Summon app.
  2. You will be asked if you like pizza. Say 'suggest food'.
  3. See state.

I hope you can test it with these files. If need another example let me know.

UPDATE: Forgot to change Alexa locale (is not used in this example but maybe you do use it and fails for you)

jrglg avatar Jun 07 '22 15:06 jrglg

Hey @aswetlow

Another example without delegating at the beggening.

This one throws an error at runtime because can't find a handler in LovePizzaComponent. It is looking the wrong component.

test_this_one_fails_at_runtime.zip

jrglg avatar Jun 07 '22 15:06 jrglg

Thank you, this was helpful.

I think I found a bug in our _mergeWith implementation: https://github.com/jovotech/jovo-framework/blob/v4/latest/platforms/platform-googleassistant/src/GoogleAssistantPlatform.ts#L71 The $session object isn't merged properly.

I might have a fix, but I'm not sure of any side effects. Will do some tests and prepare a fix...

aswetlow avatar Jun 07 '22 16:06 aswetlow

I encountered this problem as well, here are my thoughts I shared on Slack on how this could be handled:

I've a state stack that contains [ComponentA, ComponentA.ComponentB] (so I delegated from ComponentA to ComponentB). If ComponentB has UNHANDLED and I trigger an intent handled by ComponentA which has @PrioritizedOverUnhandled annotation, the request is redirected to the handler in ComponentA as expected, but the state stack becomes [ComponentA, ComponentA]. At this point, I can't access the data of the first ComponentA. The resolve object from ComponentA.ComponentB is gone as well, so I can no longer resolve the delegation and I'm stuck here. In this case, since the request is handled by an handler of a component which is part of the current state stack, wouldn't it make more sense to just pop out ComponentB from the stack? On the contrary, if the request was handled by a global handler of a root component not in the current stack, it would have made sense to clear the state stack (like this.$redirect), because we would have exited the stack scope. What are your thoughts about this? And if this is by design, what's the best way to detect this situation? I think that the same happens in the following situations as well:

  • ComponentB is a root component and not a subcomponent of ComponentA (so the initial state would be [ComponentA, ComponentB] and not [ComponentA, ComponentA.ComponentB]).
  • ComponentB doesn't have UNHANDLED and you trigger a global handler from a root component or another handler which is part of the current stack.

This problem may be linked to #1380.

acerbisgianluca avatar Jul 29 '22 13:07 acerbisgianluca

I also feel like this should be changed... Component data is not as handy as it should be.

El vie., 29 jul. 2022 15:28, Gianluca Acerbis @.***> escribió:

I encountered this problem as well, here are my thoughts I shared on Slack on how this could be handled:

I've a state stack that contains [ComponentA, ComponentA.ComponentB] (so I delegated from ComponentA to ComponentB). If ComponentB has UNHANDLED and I trigger an intent handled by ComponentA which has @PrioritizedOverUnhandled annotation, the request is redirected to the handler in ComponentA as expected, but the state stack becomes [ComponentA, ComponentA]. At this point, I can't access the data of the first ComponentA. The resolve object from ComponentA.ComponentB is gone as well, so I can no longer resolve the delegation and I'm stuck here. In this case, since the request is handled by an handler of a component which is part of the current state stack, wouldn't it make more sense to just pop out ComponentB from the stack? On the contrary, if the request was handled by a global handler of a root component not in the current stack, it would have made sense to clear the state stack (like this.$redirect), because we would have exited the stack scope. What are your thoughts about this? And if this is by design, what's the best way to detect this situation? I think that the same happens in the following situations as well:

  • ComponentB is a root component and not a subcomponent of ComponentA (so the initial state would be [ComponentA, ComponentB] and not [ComponentA, ComponentA.ComponentB]).
  • ComponentB doesn't have UNHANDLED and you trigger a global handler from a root component or another handler which is part of the current stack.

This problem may be linked to #1380 https://github.com/jovotech/jovo-framework/issues/1380.

— Reply to this email directly, view it on GitHub https://github.com/jovotech/jovo-framework/issues/1211#issuecomment-1199286802, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMTE3JCZ6ZBGET4PSHVTIU3VWPMBBANCNFSM5NH47Y4Q . You are receiving this because you were mentioned.Message ID: @.***>

jrglg avatar Jul 29 '22 18:07 jrglg