bespoken / virtual-alexa

:robot: Easily test and debug Alexa skills programmatically
https://bespoken.io
Apache License 2.0
112 stars 35 forks source link

fix(audio-player): better error message for undefined stream url #93

Closed CoreyCole closed 6 years ago

CoreyCole commented 6 years ago

There was a bug in my code where my stream url was null and I was getting a cryptic error message:

TypeError: Cannot read property 'startsWith' of null

     at AudioPlayer.<anonymous> (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:233:46)
     at step (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:32:23)
     at Object.next (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:13:53)
     at node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:7:71
     at Object.<anonymous>.__awaiter (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:3:12)
     at AudioPlayer.Object.<anonymous>.AudioPlayer.playNext (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:227:16)
     at AudioPlayer.<anonymous> (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:183:41)
     at step (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:32:23)
     at Object.next (node_modules/virtual-alexa/lib/src/audioPlayer/AudioPlayer.js:13:53)

I wasn't sure which paramter it was complaining about, so I had to look at the source code in AudioPlayer.ts.

This small check should return a more descriptive error message and let the developer know that it is the stream URL that is the problem.

codecov[bot] commented 6 years ago

Codecov Report

Merging #93 into master will increase coverage by 0.16%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   95.46%   95.63%   +0.16%     
==========================================
  Files          27       27              
  Lines        1280     1282       +2     
  Branches      180      181       +1     
==========================================
+ Hits         1222     1226       +4     
+ Misses         58       56       -2
Impacted Files Coverage Δ
src/audioPlayer/AudioPlayer.ts 79.79% <100%> (+0.41%) :arrow_up:
src/core/SkillRequest.ts 97.22% <0%> (+0.92%) :arrow_up:
src/impl/SkillInteractor.ts 100% <0%> (+1.04%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b505c70...44ef7b0. Read the comment docs.

CoreyCole commented 6 years ago

I'm not sure how to test this because in the simple audio player the stream urls are hard coded. It doesn't look like the "HTTP:" stream url check is tested either.

CoreyCole commented 6 years ago

I added a new intent that attempts to play a stream with a null url. I hope that is an OK way to test this feature.