jovotech / jovo-framework

🔈 The React for Voice and Chat: Build Apps for Alexa, Messenger, Instagram, the Web, and more
https://www.jovo.tech
Apache License 2.0
1.68k stars 309 forks source link

Unwanted Interaction between UNHANDLED, routing and order of component inclusion in app.ts #1443

Closed fbublitz closed 1 year ago

fbublitz commented 1 year ago

I'm submitting a bug? I have no idea what is happening, so i dont know if it is a bug or my usage of Jovo

Expected Behavior

the order of the components i include in app.components array (defined in app.ts) does not influence the routing results during request processing.

Current Behavior

the setup is as follows. I have one component with a global UNHANDLED handler, as the "last line of defense".

@Component()
export class GlobalUnhandled extends BaseComponent {
  @Global()
  UNHANDLED() {
    return this.$redirect(A, 'handlerInA');
  }
}

i have some other components of course, which i setup in my app.ts

const app = new App({
  components: [ A, B, GlobalUnhandled, C, D];
//... other stuff
})

I just upgraded my jovo components from 4.2.27 to 4.3.6. Now when i run this, the following happens. A request with an intent that has a global handler in component C comes in, but instead of this handler the UNHANDLED is matched by routing and executed. When i move component C in front of GlobalUnhandled in the app.ts component list, everything works fine.

Moving GlobalUnhandled to the end of the list is an obvious fix, but the behavior is very unexpected and hard to debug. I only caught it because i log an error in my UNHANDLED.

Error Log

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

Your Environment

jankoenig commented 1 year ago

Thanks for flagging @fbublitz. What a coincidence, @aswetlow and I were just talking about this when the issue came in 😅

We'll take a closer look and see how this happened!

fbublitz commented 1 year ago

Amazing :) Would really like to know how it happens, just out of curiosity.

fbublitz commented 1 year ago

Hey i found something else, that is related to this i think (and a proper bug ). i have two global handlers in two different components that handle the same intent, only one of them has an additional @If() decorator, checking on entity value

@Component()
class compA extends BaseComponent {
   @Global()
   @Intents( 'intentName' )
   handlerA() {
      // do something
   };
}

@Component()
class compB extends BaseComponent {
   @Global()
   @Intents( 'intentName' )
   @If( (jovo: Jovo) => jovo.$entities.entityType?.resolved === 'expectedValue' )
   handlerB() {
      // do something
   };
}
  1. unit test makes a request that has intentName and entityType with value expectedValue . It fails because handlerA matches instead of handlerB. According to documentation the handler that satisfies the most conditions should match.
  2. When i put compB before compA in the app.ts component array, the test succeeds. So effect of ordering shows up here too.
jrglg commented 1 year ago

This also happens to me with some CA handlers and are somehow prioritized when routing. It's difficult to override.

El jue., 3 nov. 2022 16:16, Frank B. @.***> escribió:

Hey i found something else, that is related to this i think (and a proper bug ). i have two global handlers in two different components that handle the same intent, only one of them has an additional @if https://github.com/if() decorator, checking on entity value

@Component()class compA extends BaseComponent { @Global() @Intents( 'intentName' ) handlerA() { // do something };}

@Component()class compB extends BaseComponent { @Global() @Intents( 'intentName' ) @If( (jovo: Jovo) => jovo.$entities.entityType?.resolved === 'expectedValue' ) handlerB() { // do something };}

  1. unit test makes a request that has intentName and entityType with value expectedValue . It fails because handlerA matches instead of handlerB. According to documentation the handler that satisfies the most conditions should match.
  2. When i put compB before compA in the app.ts component array, the test succeeds. So effect of ordering shows up here too.

— Reply to this email directly, view it on GitHub https://github.com/jovotech/jovo-framework/issues/1443#issuecomment-1302266353, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMTE3JFGDL7HANXGA2OI3LTWGPJNBANCNFSM6AAAAAARVFPC44 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

rmtuckerphx commented 1 year ago

@jankoenig Maybe we should add the ability to give a priority ordering to handlers that the router would use?

jankoenig commented 1 year ago

Thank you for the details @fbublitz 🎉

aswetlow commented 1 year ago

We published the update yesterday. v4.4.0