Repository: RasaHQ/rasa
Pull request: handle invalid export paths
**Proposed changes**:
- fix https://github.com/RasaHQ/rasa/issues/3812
- validate export paths in interactive learning
- added error handling in case path does not exist

**Status (please check what you already did)**:
- [x] made PR ready for code review
- [x] added some tests for the functionality
- [ ] updated the documentation
- [x] updated the changelog
- [x] reformat files using `black` (please check [Readme](https://github.com/RasaHQ/rasa_nlu#code-style) for instructions)


Problem statement:
interactive learning throws error with no output file
```
? Export stories to (if file exists, this will append the stories)
? Export NLU data to (if file exists, this will merge learned data with previous training examples)
? Export domain file to (if file exists, this will be overwritten) domain.yml
2019-06-18 08:16:35 ERROR    rasa.core.training.interactive  - An exception occurred while recording messages.
Traceback (most recent call last):
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 1367, in record_messages
    endpoint, sender_id, sender_ids, plot_file
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 885, in _predict_till_next_listen
    action_name, policy, confidence, predictions, endpoint, sender_id
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 1024, in _validate_action
    is_correct = await _ask_questions(question, sender_id, endpoint)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 312, in _ask_questions
    should_retry = await _ask_if_quit(sender_id, endpoint)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 591, in _ask_if_quit
    await _write_data_to_file(sender_id, endpoint)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 566, in _write_data_to_file
    await _write_stories_to_file(story_path, events)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 764, in _write_stories_to_file
    with open(export_story_path, append_write, encoding="utf-8") as f:
FileNotFoundError: [Errno 2] No such file or directory: ''
2019-06-18 08:16:35 ERROR    asyncio  - Task exception was never retrieved
future: <Task finished coro=<_serve_application.<locals>.run_interactive_io() done, defined at /Users/erohmensing/rasa/rasa/core/training/interactive.py:1409> exception=FileNotFoundError(2, 'No such file or directory')>
Traceback (most recent call last):
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 1416, in run_interactive_io
    sender_id=uuid.uuid4().hex,
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 1367, in record_messages
    endpoint, sender_id, sender_ids, plot_file
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 885, in _predict_till_next_listen
    action_name, policy, confidence, predictions, endpoint, sender_id
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 1024, in _validate_action
    is_correct = await _ask_questions(question, sender_id, endpoint)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 312, in _ask_questions
    should_retry = await _ask_if_quit(sender_id, endpoint)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 591, in _ask_if_quit
    await _write_data_to_file(sender_id, endpoint)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 566, in _write_data_to_file
    await _write_stories_to_file(story_path, events)
  File "/Users/erohmensing/rasa/rasa/core/training/interactive.py", line 764, in _write_stories_to_file
    with open(export_story_path, append_write, encoding="utf-8") as f:
FileNotFoundError: [Errno 2] No such file or directory: ''
```
"If this file exists, this will append the stories" -- if I don't pass anything (i.e. ''), it shouldn't throw an error, it just shouldn't append my stories files

Hints from issue discussion:

Commit message: fixed types

Patch to review:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 8afff776892a..cd2bc03061f3 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -26,6 +26,7 @@ Removed
 -------
 - revert the stripping of trailing slashes in endpoint URLs since this can lead to
   problems in case the trailing slash is actually wanted
+- validate export paths in interactive learning
 
 Fixed
 -----
diff --git a/rasa/core/training/interactive.py b/rasa/core/training/interactive.py
index 646256f1cfc2..47fa311183eb 100644
--- a/rasa/core/training/interactive.py
+++ b/rasa/core/training/interactive.py
@@ -655,22 +655,37 @@ async def _request_action_from_user(
 def _request_export_info() -> Tuple[Text, Text, Text]:
     """Request file path and export stories & nlu data to that path"""
 
+    def is_not_empty(path: Text) -> bool:
+        return path is not None and path != ""
+
+    def validate_story_export_file(path: Text) -> bool:
+        return is_not_empty(path) and path.endswith(".md")
+
+    def validate_nlu_export_file(path: Text) -> bool:
+        return is_not_empty(path) and (path.endswith(".md") or path.endswith(".json"))
+
+    def validate_domain_export_file(path: Text) -> bool:
+        return is_not_empty(path) and (path.endswith(".yaml") or path.endswith(".yml"))
+
     # export training data and quit
     questions = questionary.form(
         export_stories=questionary.text(
             message="Export stories to (if file exists, this "
             "will append the stories)",
             default=PATHS["stories"],
+            validate=validate_story_export_file,
         ),
         export_nlu=questionary.text(
             message="Export NLU data to (if file exists, this will "
             "merge learned data with previous training examples)",
             default=PATHS["nlu"],
+            validate=validate_nlu_export_file,
         ),
         export_domain=questionary.text(
             message="Export domain file to (if file exists, this "
             "will be overwritten)",
             default=PATHS["domain"],
+            validate=validate_domain_export_file,
         ),
     )
 
@@ -778,8 +793,7 @@ async def _write_nlu_to_file(
     try:
         previous_examples = load_data(export_nlu_path)
     except Exception as e:
-        logger.exception("An exception occurred while trying to load the NLU data.")
-
+        logger.error("An exception occurred while trying to load the NLU data.")
         export_nlu_path = questionary.text(
             message="Could not load existing NLU data, please "
             "specify where to store NLU data learned in "
diff --git a/rasa/nlu/training_data/loading.py b/rasa/nlu/training_data/loading.py
index 0b3f76a08a8c..87a179f57e1a 100644
--- a/rasa/nlu/training_data/loading.py
+++ b/rasa/nlu/training_data/loading.py
@@ -1,5 +1,7 @@
 import json
 import logging
+import os
+
 import requests
 import typing
 from typing import Optional, Text
@@ -52,6 +54,9 @@ def load_data(resource_name: Text, language: Optional[Text] = "en") -> "Training
     Merges them if loaded from disk and multiple files are found."""
     from rasa.nlu.training_data import TrainingData
 
+    if not os.path.exists(resource_name):
+        raise ValueError("File '{}' does not exist.".format(resource_name))
+
     files = utils.list_files(resource_name)
     data_sets = [_load(f, language) for f in files]
     data_sets = [ds for ds in data_sets if ds]
@@ -112,9 +117,9 @@ def _load(filename: Text, language: Optional[Text] = "en") -> Optional["Training
 
     fformat = _guess_format(filename)
     if fformat == UNK:
-        raise ValueError("Unknown data format for file {}".format(filename))
+        raise ValueError("Unknown data format for file '{}'.".format(filename))
 
-    logger.info("Training data format of {} is {}".format(filename, fformat))
+    logger.info("Training data format of '{}' is '{}'.".format(filename, fformat))
     reader = _reader_factory(fformat)
 
     if reader:
@@ -126,8 +131,10 @@ def _load(filename: Text, language: Optional[Text] = "en") -> Optional["Training
 def _guess_format(filename: Text) -> Text:
     """Applies heuristics to guess the data format of a file."""
     guess = UNK
-    content = rasa.utils.io.read_file(filename)
+
+    content = ""
     try:
+        content = rasa.utils.io.read_file(filename)
         js = json.loads(content)
     except ValueError:
         if any([marker in content for marker in _markdown_section_markers]):
diff --git a/rasa/utils/io.py b/rasa/utils/io.py
index 36346536d926..044723b02a7a 100644
--- a/rasa/utils/io.py
+++ b/rasa/utils/io.py
@@ -120,8 +120,12 @@ def read_yaml(content: Text) -> Union[List[Any], Dict[Text, Any]]:
 
 def read_file(filename: Text, encoding: Text = "utf-8") -> Any:
     """Read text from a file."""
-    with open(filename, encoding=encoding) as f:
-        return f.read()
+
+    try:
+        with open(filename, encoding=encoding) as f:
+            return f.read()
+    except FileNotFoundError:
+        raise ValueError("File '{}' does not exist.".format(filename))
 
 
 def read_json_file(filename: Text) -> Any:
diff --git a/tests/nlu/base/test_training_data.py b/tests/nlu/base/test_training_data.py
index 07f36edf385d..249a6bb1554c 100644
--- a/tests/nlu/base/test_training_data.py
+++ b/tests/nlu/base/test_training_data.py
@@ -4,12 +4,13 @@
 import tempfile
 from jsonschema import ValidationError
 
-from rasa.nlu import training_data, utils
+from rasa.nlu import training_data
 from rasa.nlu.convert import convert_training_data
 from rasa.nlu.extractors.mitie_entity_extractor import MitieEntityExtractor
 from rasa.nlu.tokenizers.whitespace_tokenizer import WhitespaceTokenizer
 from rasa.nlu.training_data.formats import MarkdownReader
 from rasa.nlu.training_data.formats.rasa import validate_rasa_nlu_data
+from rasa.nlu.training_data.loading import _guess_format, UNK, load_data
 from rasa.nlu.training_data.util import get_file_format
 import rasa.utils.io as io_utils
 
@@ -529,3 +530,12 @@ def test_get_file_format():
 
     with pytest.raises(AttributeError):
         get_file_format(None)
+
+
+def test_guess_format_from_non_existing_file_path():
+    assert _guess_format("not existing path") == UNK
+
+
+def test_load_data_from_non_existing_file():
+    with pytest.raises(ValueError):
+        load_data("some path")
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 24c383312ceb..364266eee224 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -118,3 +118,8 @@ def test_warning_for_base_paths_with_trailing_slash(caplog):
         assert concat_url(test_path, None) == test_path
 
     assert len(caplog.records) == 1
+
+
+def test_read_file_with_not_existing_path():
+    with pytest.raises(ValueError):
+        rasa.utils.io.read_file("some path")


Code review comments:
diff_hunk: @@ -778,8 +793,7 @@ def _collect_actions(events: List[Dict[Text, Any]]) -> List[Dict[Text, Any]]:
     try:
         previous_examples = load_data(export_nlu_path)
     except Exception as e:
path: rasa/core/training/interactive.py
text: @user1:
I feel like in this exception we should just output to the file given, since we've already validated it. imo this flow:
```
? Do you want to stop?  Export & Quit
? Export stories to (if file exists, this will append the stories) data/stories2.md
? Export NLU data to (if file exists, this will merge learned data with previous training examples) data/nlu2.md
? Export domain file to (if file exists, this will be overwritten) domain2.yml
2019-06-28 16:06:48 ERROR    rasa.core.training.interactive  - An exception occurred while trying to load the NLU data.
? Could not load existing NLU data, please specify where to store NLU data learned in this session (this will overwrite any existing file). Unknown data format for file 'data/nlu2.md'. data/nlu_interactive.md
```
is strange (`data/nlu_interactive.md` is the default that auto-populates). I've already told it where to put it, and it says `if file exists` it will append, so if it doesn't it should just create one and write it without asking again. This only happens for NLU export, not the other two

@author:
makes sense, I agree :+1:

@user1:
@author was this handled? i’m still running into it 

@author:
Thanks for linking this. Didn't find it when searching for it. will handle it.

diff_hunk: @@ -655,22 +655,37 @@ def _slot_history(tracker_dump: Dict[Text, Any]) -> List[Text]:
 def _request_export_info() -> Tuple[Text, Text, Text]:
     """Request file path and export stories & nlu data to that path"""
 
+    def is_not_empty(path: Text) -> bool:
+        return path is not None and path != ""
+
+    def validate_story_export_file(path: Text) -> bool:
+        return is_not_empty(path) and path.endswith(".md")
+
+    def validate_nlu_export_file(path: Text) -> bool:
+        return is_not_empty(path) and (path.endswith(".md") or path.endswith(".json"))
+
+    def validate_domain_export_file(path: Text) -> bool:
+        return is_not_empty(path) and (path.endswith(".yaml") or path.endswith(".yml"))
+
     # export training data and quit
     questions = questionary.form(
         export_stories=questionary.text(
             message="Export stories to (if file exists, this "
             "will append the stories)",
             default=PATHS["stories"],
+            validate=validate_story_export_file,
path: rasa/core/training/interactive.py
text: @user1:
Maybe we could even replace these with e.g.:
```
validate=Validator.from_callable(validate_story_export_file, error_message='Stories file must end in .md'), 
```
to make use of more descriptive error msgs than `invalid input`

Source here: https://github.com/prompt-toolkit/python-prompt-toolkit/blob/c7851c09ffbfa7d06458408abee77ab6f548852d/prompt_toolkit/validation.py#L84

@author:
Cool suggestion, will do!

@author:
had to do it a bit differently since questionary doesn't support objects of this type. Will do a pr there :-)

diff_hunk: @@ -655,22 +655,37 @@ def _slot_history(tracker_dump: Dict[Text, Any]) -> List[Text]:
 def _request_export_info() -> Tuple[Text, Text, Text]:
     """Request file path and export stories & nlu data to that path"""
 
+    def is_not_empty(path: Text) -> bool:
+        return path is not None and path != ""
+
+    def validate_story_export_file(path: Text) -> bool:
+        return is_not_empty(path) and path.endswith(".md")
+
+    def validate_nlu_export_file(path: Text) -> bool:
+        return is_not_empty(path) and (path.endswith(".md") or path.endswith(".json"))
path: rasa/core/training/interactive.py
text: @user1:
Tested this with a json, but it still writes in md, so should either be able to export json or take out this second part