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

Handle Mimic missing properly #2718

Merged
merged 3 commits into from Nov 16, 2020

Conversation

forslund
Copy link
Collaborator

@forslund forslund commented Oct 10, 2020

Description

I got an error report on chat with the following error:

Traceback (most recent call last):
  File "/home/roy/mycroft-core/mycroft/tts/tts.py", line 519, in create
    tts.validator.validate()
  File "/home/roy/mycroft-core/mycroft/tts/tts.py", line 435, in validate
    self.validate_connection()
  File "/home/roy/mycroft-core/mycroft/tts/mimic_tts.py", line 189, in validate_connection
    LOG.info("Failed to find mimic at: " + BIN)
TypeError: can only concatenate str (not "NoneType") to str
2020-10-08 16:49:47.127 | ERROR    | 51541 | __main__:on_error:34 | Audio service failed to launch (TypeError('can only concatenate str (not "NoneType") to str')).

So the error message in the exception handler throws an exception making the real exception not show.

I also went through the module with pylint and fixed the issues that could be fixed without breaking backwards compatibility.

How to test

Make sure Mimic still works and unittests works. You can also set BIN to None and check that the correct exception is shown.

Contributor license agreement signed?

CLA [ Yes ]

If Mimic was not found at all the LOG message would raise an exception,
this fixes that issue and cleans up Exception chain
@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 10, 2020
@forslund forslund added the Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained. label Oct 10, 2020
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling krisgesling added this to Inbox in Roadmap via automation Oct 12, 2020
@krisgesling krisgesling moved this from Inbox to Next in Roadmap Oct 12, 2020
@krisgesling
Copy link
Contributor

Still a possible error. Steps to reproduce:

  1. remove mimic directory
  2. launch Mycroft with internet
  3. disconnect internet
  4. speak a request
Traceback (most recent call last):
  File "/home/gez/mycroft-core/mycroft/audio/speech.py", line 95, in handle_speak
    mute_and_speak(chunk, ident, listen)
  File "/home/gez/mycroft-core/mycroft/audio/speech.py", line 132, in mute_and_speak
    mimic_fallback_tts(utterance, ident, listen)
  File "/home/gez/mycroft-core/mycroft/audio/speech.py", line 148, in mimic_fallback_tts
    tts.execute(utterance, ident, listen)
  File "/home/gez/mycroft-core/mycroft/tts/tts.py", line 321, in execute
    self._execute(sentence, ident, listen)
  File "/home/gez/mycroft-core/mycroft/tts/tts.py", line 351, in _execute
    wav_file, phonemes = self.get_tts(sentence, wav_file)
  File "/home/gez/mycroft-core/mycroft/tts/mimic_tts.py", line 159, in get_tts
    '-t', sentence])
  File "/usr/lib/python3.6/subprocess.py", line 356, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.6/subprocess.py", line 423, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.6/subprocess.py", line 729, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.6/subprocess.py", line 1278, in _execute_child
    executable = os.fsencode(executable)
  File "/usr/lib/python3.6/os.py", line 800, in fsencode
    filename = fspath(filename)  # Does type-checking of `filename`.
TypeError: expected str, bytes or os.PathLike object, not NoneType

Strangely in get_tts() neither sentence nor wav_file are None or NoneType values.

@forslund
Copy link
Collaborator Author

Likely the mimic path is None... The issue is that the fallback_tts object shouldn't be created at all in this case since the validation should fail (since mimic doesn't exist) will investigate

Verify that the fallback object can execute when creating it
@forslund forslund force-pushed the bugfix/mimic-missing-exceptions branch from ffe3ef5 to 1ae6271 Compare November 14, 2020 10:45
@forslund
Copy link
Collaborator Author

So the root issue seem to be that the fallback_tts was never validated. I moved things around a little and added the validation for the fallback.

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

1 similar comment
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks Ake!

@krisgesling krisgesling merged commit b9c974b into MycroftAI:dev Nov 16, 2020
Roadmap automation moved this from Next to Done Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained.
Projects
Roadmap
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants