Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 of 4 tasks
jtfell opened this issue Feb 1, 2022 · 10 comments
Open
1 of 4 tasks
Labels

Comments

@jtfell
Copy link

jtfell commented Feb 1, 2022

I'm submitting a...

  • 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
@jankoenig
Copy link
Member

jankoenig commented Feb 10, 2022

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.

@rmtuckerphx
Copy link
Contributor

@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": {}
     }
   }
 }

@jankoenig
Copy link
Member

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?

@jrglg
Copy link
Contributor

jrglg commented Jun 6, 2022

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.

@aswetlow
Copy link
Member

aswetlow commented Jun 7, 2022

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

@jrglg
Copy link
Contributor

jrglg commented Jun 7, 2022

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
Copy link
Contributor

jrglg commented Jun 7, 2022

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

@aswetlow
Copy link
Member

aswetlow commented Jun 7, 2022

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 added a commit that referenced this issue Jun 8, 2022
🐛 Fix state stack merge on Google Assistant #1211
rubenaeg pushed a commit to die-lautmaler/jovo-framework that referenced this issue Jun 13, 2022
@acerbisgianluca
Copy link
Contributor

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.

@jrglg
Copy link
Contributor

jrglg commented Jul 29, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants