Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

Why isn't sendExceptionEncountered called when directive isn't handled #6

Closed
garmin-holder opened this issue Apr 25, 2017 · 5 comments

Comments

@garmin-holder
Copy link

No exception is sent in DirectiveSequencer::onDirective when the directive is unhandled, but it seems like UNSUPPORTED_OPERATION should be sent in that case.

@JamieMeyers
Copy link
Contributor

Hi @garmin-holder,

Great question. I believe you are talking about the shutting down case (since in the nullptr case we can't satisfy the ExceptionEncountered event requirements). In this case, we're purposefully ignoring incoming messages. Per the ExceptionEncountered Event, "Your client must send this event when it is unable to execute a directive from AVS." These are exceptional events and should only be sent when there is a failure in attempting to execute, not when we choose to ignore the directive. We're likely able to execute it, we're just unwilling to do so as ADSL is shutting down.

Does that answer your question?

Thanks,
Jamie

@garmin-holder
Copy link
Author

Thanks for the prompt response. I didn't do a good job of stating my reason for asking. To try to get a handle on the exception reporting strategy and how the SDK participates in particular, I commented out one of the handle directive registrations in the client and then sent that directive. I was surprised so see that it didn't report an exception when the system couldn't handle that directive. I notice that that exception is sent in the shouldProcessDirective, but not where I assume the actual processing is dispatched (onDirective).

The bigger question is if the client should rely on the SDK to report exceptions, or if the client must report all exceptions, including speech failed, which is reported back to the sequencer already?

Thanks,
Alan

@scotthea-amazon
Copy link
Contributor

scotthea-amazon commented Apr 25, 2017

Hi @garmin-holder,

The SDK should be calling sendExceptionEncountered for uhandled directives. The 0.2 code DirectiveSequencer is currently broken in this respect. Just as you reported, it drops the the unhandled directive instead. It also calls sendExceptionEncountered() in a couple of places it should not (during shutdown and when onDirective is called while directive handlers are being set). We have a fix in progress for each of these errors.

The SDK should only be calling sendExceptionEncountered() for the case where a directive could not be parsed or when no handler is registered for a directive.

Thanks,
-SWH

@JamieMeyers
Copy link
Contributor

Hi @garmin-holder, my apologies, I slightly misspoke above because I was looking at the newer code. @scotthea-amazon was correct in that in the 0.2 release, we currently have a few bugs surrounding the exception. These are fixed in the next update, hence my last comment.

To answer you question surrounding who owns sending ExceptionEncountered messages, it is split depending on the stage in the directive's lifecycle.

  • The SDK is responsible for sending ExceptionEncountered messages on failures before ADSL asks the DirectiveHandler/client to handle the Directive.
  • The DirectiveHandler/client is responsible for sending ExceptionEncountered messages on failures during prehandling or anytime a failure happens during or after handling.

@scotthea-amazon
Copy link
Contributor

This is addressed in the 0.2.1 version just released.

@JamieMeyers JamieMeyers added the bug label Jun 2, 2017
@baddemiya baddemiya mentioned this issue Feb 17, 2018
2 tasks
@adeb adeb mentioned this issue Apr 8, 2018
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants