search

alexa / alexa-skills-kit-sdk-for-nodejs

The Alexa Skills Kit SDK for Node.js helps you get a skill up and running quickly, letting you focus on skill logic instead of boilerplate code.
Apache License 2.0
3.12k stars 735 forks source link

TypeError in getSupportedInterfaces #709

Closed igilham closed 2 years ago

igilham commented 2 years ago

The utility function getSupportedInterfaces contains a TypeError for certain types of requests.

https://github.com/alexa/alexa-skills-kit-sdk-for-nodejs/blob/ef6b97e9deaeb902856c33e570f56da8a2ff998a/ask-sdk-core/lib/util/RequestEnvelopeUtils.ts#L274

This code naïvely ignores whether or not the device field is defined, throwing the error for requests that do not include it.

I've seen this cause a skill to crash in skill event (e.g. AlexaSkillEvent.SkillEnabled) and account linking (e.g. AlexaSkillEvent.SkillAccountLinked) requests, which do not define the device property.

igilham commented 2 years ago

Fixing this bug is trivial but fixing the test suite is more complex.

The tests only exercise one type of RequestEnvelope, a LaunchRequest. There are several other types of requests which have different properties defined so there's quite a large gap in the test coverage for the core package of the SDK.

That wasn't quite correct. There's one mock but the tests modify it in-place to cover different types of requests. This seems a little fragile to me but it's a reasonable way to do it.

There is a missing test case for calling getSupportedInterfaces with a request envelope with devices undefined.

igilham commented 2 years ago

Please note that in my own testing, using strict mode in Typescript makes this error impossible. It may be worth considering tighter compiler options to reduce this sort of thing in the future.