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

Crash when snips.slots is None #28

Open
franc6 opened this issue Mar 7, 2019 · 4 comments
Open

Crash when snips.slots is None #28

franc6 opened this issue Mar 7, 2019 · 4 comments

Comments

@franc6
Copy link
Contributor

franc6 commented Mar 7, 2019

If the searchWeatherForecast without any slots, snips.slots is None, which leads to a cash at line 79 of action-own.py when access snips.slots.forecast_start_datetime. I believe this is related to changes in hermes_python 0.3.0 and later.
A quick fix is to verify that snips.slots is not None at line 79 and in similar locations.

@bors-ltd
Copy link

bors-ltd commented Mar 9, 2019

Just got it:

Mar  9 19:44:14 snips-base snips-skill-server[3604]: INFO:snips_skill_server_lib::runner: [owm][err]     if snips.slots.forecast_start_datetime:
Mar  9 19:44:14 snips-base snips-skill-server[3604]: INFO:snips_skill_server_lib::runner: [owm][err] AttributeError: 'NoneType' object has no attribute 'forecast_start_datetime'

@anthonyray
Copy link
Contributor

Do you have the latest version of snips-skill-owm running ?

@franc6
Copy link
Contributor Author

franc6 commented Mar 11, 2019

Yes. I verified that before opening the issue. Please note that I've only seen the problem with hermes-python 0.3.2, and 0.3.3. Versions 0.3.0 and 0.3.1 had too many other problems for me to have noticed if the problem existed with them.

Also, please note that the generated intent must not have any slots at all. If, for example, you ask for the weather in a specific location, but don't specify when, it will work as expected, because snips.slots will not be None, and the check for snips.slots.forecast_start_datetime will not cause an error. What should happen in this case is that you get the current weather for the location defined in config.ini as default_location in the secret section.

I suspect that in the past, the IntentMessage object's slots member was an empty list when no slots were filled, and the newer code doesn't do this, instead simply setting the slots member to None. Which is probably better behavior. :)

Please note, too, this happens with or without the fix for #26.

@bors-ltd
Copy link

I sure do, this was a brand new installation this week-end.

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

No branches or pull requests

3 participants