An issue with ^: syntax checks should happen after ^Continue commands are handled

[arashsa]

+ ok
^ 1) test
^ 2) test

(edit by @kirsle: fenced code block for RiveScript snippet)

If the above is in a file an error occurs.

  File "/Users/arashsaidi/.pyenv/versions/convertelligence-bot/lib/python3.5/site-packages/rivescript/rivescript.py", line 1002, in reply
    return self._brain.reply(user, msg, errors_as_replies)
  File "/Users/arashsaidi/.pyenv/versions/convertelligence-bot/lib/python3.5/site-packages/rivescript/brain.py", line 82, in reply
    reply = self._getreply(user, msg, ignore_object_errors=errors_as_replies)
  File "/Users/arashsaidi/.pyenv/versions/convertelligence-bot/lib/python3.5/site-packages/rivescript/brain.py", line 269, in _getreply
    regexp = self.reply_regexp(user, pattern)
  File "/Users/arashsaidi/.pyenv/versions/convertelligence-bot/lib/python3.5/site-packages/rivescript/brain.py", line 503, in reply_regexp
    return re.compile(r'^' + regexp + r'$')
  File "/Users/arashsaidi/.pyenv/versions/3.5.2/lib/python3.5/re.py", line 224, in compile
    return _compile(pattern, flags)
  File "/Users/arashsaidi/.pyenv/versions/3.5.2/lib/python3.5/re.py", line 293, in _compile
    p = sre_compile.compile(pattern, flags)
  File "/Users/arashsaidi/.pyenv/versions/3.5.2/lib/python3.5/sre_compile.py", line 536, in compile
    p = sre_parse.parse(p, flags)
  File "/Users/arashsaidi/.pyenv/versions/3.5.2/lib/python3.5/sre_parse.py", line 834, in parse
    raise source.error("unbalanced parenthesis")
sre_constants.error: unbalanced parenthesis at position 4

[kirsle]

You're extending a +Trigger with those ^Continue commands, so an error is expected. 😀

The code is equivalent to (with the default concat mode):

+ ok1) test2) test

What's not expected, though, is that the error came from the Python regexp module and not my own code. In the RiveScript parser, I syntax check triggers by counting opening and closing brackets and check for obvious mismatches (it's still possible to write a trigger that has all the brackets match up but that is still an invalid regexp, so my bracket counting code is just a quick sanity check to prevent obvious errors).

The syntax check currently happens before ^Continue commands are joined together, so it should be moved to after that. Then you'd get a parse-time error from RiveScript rather than a reply-time error from Python. :wink:

[arashsa]

Yes, badly formulated by me. There should be a warning! 😄

Is there a way to ensure that similar things do not happen for other cases? It just seems very scary if one use it in production that similar things can happen and a python program would crash.

[kirsle]

I try to detect and handle as many problematic cases as I can, but every now and then something would slip through the cracks. Regular expressions in general are complicated, so my syntax checking does a quick look to detect any obvious things that could go wrong, but it wouldn't raise an error on these sorts of triggers:

+ )invalid( parenthesis
+ (or <this) thing>

...because the brackets are all balanced, but they'd result in invalid regular expressions. I don't know of any better ("better" defined as being concise, non-messy source code) to validate a regexp apart from actually attempting to use it as a regexp. Python has slow regular expressions in general and RiveScript aggressively caches as many as it can up front (after parse time), so testing every trigger regexp could add some latency during parsing to try each trigger it sees. There's also a regexp that tests regexps, but that's getting into crazy territory. :wink:

The safest way for an end user to ensure that an uncaught exception in RiveScript doesn't crash your whole program is to put calls to RiveScript in a try/except block.

try:
    reply = rs.reply(username, message)
except Exception as e:
    log.exception("Unexpected exception getting a reply: {}".format(e))
    reply = "An internal error has occurred and has been reported to my botmaster."

[htdinh]

Hi @kirsle, I also find the continuation example in the Tutorial is not clear enough that we don't apply continuation to Trigger.

That's another story. From testing, the triggers seem understand the continuation (works with message goodmorning, good night, have a good night but not with good evening).

+ good
^ morning
- Goodmorming! 

+ good
^ \sevening
- Good evening! 

+ [have a] good
^ \snight
- Good night! 

Do you want to have that feature (Trigger understands continuation)? If so I can take a look and develop this.

[kirsle]

Go ahead. :) Syntax checks should happen after ^Continues