Prevent functions from throwing loop control flow exceptions; convert them into StormRuntimeError. SYN-8397

changes/b4642a502f49353787b4f6632a6a6566.yaml

@@ -0,0 +1,9 @@
+---
+desc: Fixed an issue with the Storm loop and generator keywords, ``continue``, ``break``,
+  and ``stop``. Using these keywords to exit a function will now raise a ``StormRuntimeError``
+  exception. Using these keywords to tear down the Storm runtime will now emit an
+  ``err`` message with the type ``StormRuntimeError`` and a message indicating the
+  invalid use of the keywords.
+prs: []
+type: bug
+...

synapse/cortex.py

@@ -5910,7 +5910,6 @@ async def storm(self, text, opts=None):
         opts = self._initStormOpts(opts)
 
         if self.stormpool is not None and opts.get('mirror', True):
-            extra = await self.getLogExtra(text=text)
             proxy = await self._getMirrorProxy(opts)
 
             if proxy is not None:
@@ -5930,7 +5929,7 @@ async def storm(self, text, opts=None):
 
                 except s_exc.TimeOut:
                     mesg = 'Timeout waiting for query mirror, running locally instead.'
-                    logger.warning(mesg)
+                    logger.warning(mesg, extra=extra)
 
         if (nexsoffs := opts.get('nexsoffs')) is not None:
             if not await self.waitNexsOffs(nexsoffs, timeout=opts.get('nexstimeout')):
@@ -5945,7 +5944,6 @@ async def callStorm(self, text, opts=None):
         opts = self._initStormOpts(opts)
 
         if self.stormpool is not None and opts.get('mirror', True):
-            extra = await self.getLogExtra(text=text)
             proxy = await self._getMirrorProxy(opts)
 
             if proxy is not None:
@@ -5962,7 +5960,7 @@ async def callStorm(self, text, opts=None):
                     return await proxy.callStorm(text, opts=mirropts)
                 except s_exc.TimeOut:
                     mesg = 'Timeout waiting for query mirror, running locally instead.'
-                    logger.warning(mesg)
+                    logger.warning(mesg, extra=extra)
 
         if (nexsoffs := opts.get('nexsoffs')) is not None:
             if not await self.waitNexsOffs(nexsoffs, timeout=opts.get('nexstimeout')):
@@ -5975,7 +5973,6 @@ async def exportStorm(self, text, opts=None):
         opts = self._initStormOpts(opts)
 
         if self.stormpool is not None and opts.get('mirror', True):
-            extra = await self.getLogExtra(text=text)
             proxy = await self._getMirrorProxy(opts)
 
             if proxy is not None:
@@ -5995,7 +5992,7 @@ async def exportStorm(self, text, opts=None):
 
                 except s_exc.TimeOut:
                     mesg = 'Timeout waiting for query mirror, running locally instead.'
-                    logger.warning(mesg)
+                    logger.warning(mesg, extra=extra)
 
         if (nexsoffs := opts.get('nexsoffs')) is not None:
             if not await self.waitNexsOffs(nexsoffs, timeout=opts.get('nexstimeout')):

synapse/exc.py

@@ -58,7 +58,7 @@ def setdefault(self, name, valu):
 
     def update(self, items: dict):
         '''Update multiple items in the errinfo dict at once.'''
-        self.errinfo.update(**items)
+        self.errinfo.update(items)
         self._setExcMesg()
 
 class StormRaise(SynErr):

synapse/lib/ast.py

@@ -1020,13 +1020,13 @@ async def run(self, runt, genr):
                             yield item
 
                     except s_stormctrl.StormBreak as e:
-                        if e.item is not None:
-                            yield e.item
+                        if (eitem := e.get('item')) is not None:
+                            yield eitem
                         break
 
                     except s_stormctrl.StormContinue as e:
-                        if e.item is not None:
-                            yield e.item
+                        if (eitem := e.get('item')) is not None:
+                            yield eitem
                         continue
 
                     finally:
@@ -1079,13 +1079,13 @@ async def run(self, runt, genr):
                             yield jtem
 
                     except s_stormctrl.StormBreak as e:
-                        if e.item is not None:
-                            yield e.item
+                        if (eitem := e.get('item')) is not None:
+                            yield eitem
                         break
 
                     except s_stormctrl.StormContinue as e:
-                        if e.item is not None:
-                            yield e.item
+                        if (eitem := e.get('item')) is not None:
+                            yield eitem
                         continue
 
                     finally:
@@ -1109,13 +1109,13 @@ async def run(self, runt, genr):
                         await asyncio.sleep(0)
 
                 except s_stormctrl.StormBreak as e:
-                    if e.item is not None:
-                        yield e.item
+                    if (eitem := e.get('item')) is not None:
+                        yield eitem
                     break
 
                 except s_stormctrl.StormContinue as e:
-                    if e.item is not None:
-                        yield e.item
+                    if (eitem := e.get('item')) is not None:
+                        yield eitem
                     continue
 
                 finally:
@@ -1133,13 +1133,13 @@ async def run(self, runt, genr):
                         await asyncio.sleep(0)
 
                 except s_stormctrl.StormBreak as e:
-                    if e.item is not None:
-                        yield e.item
+                    if (eitem := e.get('item')) is not None:
+                        yield eitem
                     break
 
                 except s_stormctrl.StormContinue as e:
-                    if e.item is not None:
-                        yield e.item
+                    if (eitem := e.get('item')) is not None:
+                        yield eitem
                     continue
 
                 finally:
@@ -4724,9 +4724,9 @@ async def run(self, runt, genr):
             yield _
 
         async for node, path in genr:
-            raise s_stormctrl.StormBreak(item=(node, path))
+            raise self.addExcInfo(s_stormctrl.StormBreak(item=(node, path)))
 
-        raise s_stormctrl.StormBreak()
+        raise self.addExcInfo(s_stormctrl.StormBreak())
 
 class ContinueOper(AstNode):
 
@@ -4737,9 +4737,9 @@ async def run(self, runt, genr):
             yield _
 
         async for node, path in genr:
-            raise s_stormctrl.StormContinue(item=(node, path))
+            raise self.addExcInfo(s_stormctrl.StormContinue(item=(node, path)))
 
-        raise s_stormctrl.StormContinue()
+        raise self.addExcInfo(s_stormctrl.StormContinue())
 
 class IfClause(AstNode):
     pass
@@ -4842,8 +4842,8 @@ class Stop(Oper):
     async def run(self, runt, genr):
         for _ in (): yield _
         async for node, path in genr:
-            raise s_stormctrl.StormStop()
-        raise s_stormctrl.StormStop()
+            raise self.addExcInfo(s_stormctrl.StormStop())
+        raise self.addExcInfo(s_stormctrl.StormStop())
 
 class FuncArgs(AstNode):
     '''
@@ -4987,9 +4987,16 @@ async def callfunc(self, runt, argdefs, args, kwargs, funcpath):
                         await asyncio.sleep(0)
 
                     return None
-
                 except s_stormctrl.StormReturn as e:
                     return e.item
+                except s_stormctrl.StormLoopCtrl as e:
+                    mesg = f'function {self.name} - Loop control statement "{e.statement}" used outside of a loop.'
+                    raise self.addExcInfo(s_exc.StormRuntimeError(mesg=mesg, function=self.name,
+                                                                  statement=e.statement)) from e
+                except s_stormctrl.StormGenrCtrl as e:
+                    mesg = f'function {self.name} - Generator control statement "{e.statement}" used outside of a generator function.'
+                    raise self.addExcInfo(s_exc.StormRuntimeError(mesg=mesg, function=self.name,
+                                                                  statement=e.statement)) from e
 
         async def genr():
             async with runt.getSubRuntime(self.kids[2], opts=opts) as subr:
@@ -5009,5 +5016,9 @@ async def genr():
                                 yield node, path
                 except s_stormctrl.StormStop:
                     return
+                except s_stormctrl.StormLoopCtrl as e:
+                    mesg = f'function {self.name} - Loop control statement "{e.statement}" used outside of a loop.'
+                    raise self.addExcInfo(s_exc.StormRuntimeError(mesg=mesg, function=self.name,
+                                                                  statement=e.statement)) from e
 
         return genr()

synapse/lib/stormctrl.py

@@ -1,9 +1,91 @@
 class StormCtrlFlow(Exception):
+    '''
+    Base class all StormCtrlFlow exceptions derive from.
+    '''
+    def __init__(self):
+        raise NotImplementedError
+
+class _SynErrMixin(Exception):
+    '''
+    An exception mixin to give some control flow classes functionality like SynErr.
+    '''
+    def __init__(self, *args, **info):
+        self.errinfo = info
+        Exception.__init__(self, self._getExcMsg())
+
+    def _getExcMsg(self):
+        props = sorted(self.errinfo.items())
+        displ = ' '.join(['%s=%r' % (p, v) for (p, v) in props])
+        return '%s: %s' % (self.__class__.__name__, displ)
+
+    def _setExcMesg(self):
+        '''Should be called when self.errinfo is modified.'''
+        self.args = (self._getExcMsg(),)
+
+    def __setstate__(self, state):
+        '''Pickle support.'''
+        super(StormCtrlFlow, self).__setstate__(state)
+        self._setExcMesg()
+
+    def items(self):
+        return {k: v for k, v in self.errinfo.items()}
+
+    def get(self, name, defv=None):
+        '''
+        Return a value from the errinfo dict.
+
+        Example:
+
+            try:
+                foothing()
+            except SynErr as e:
+                blah = e.get('blah')
+
+        '''
+        return self.errinfo.get(name, defv)
+
+    def set(self, name, valu):
+        '''
+        Set a value in the errinfo dict.
+        '''
+        self.errinfo[name] = valu
+        self._setExcMesg()
+
+    def setdefault(self, name, valu):
+        '''
+        Set a value in errinfo dict if it is not already set.
+        '''
+        if name in self.errinfo:
+            return
+        self.errinfo[name] = valu
+        self._setExcMesg()
+
+    def update(self, items: dict):
+        '''Update multiple items in the errinfo dict at once.'''
+        self.errinfo.update(items)
+        self._setExcMesg()
+
+class StormLoopCtrl(_SynErrMixin):
+    # Control flow statements for WHILE and FOR loop control
+    statement = ''
+
+class StormGenrCtrl(_SynErrMixin):
+    # Control flow statements for GENERATOR control
+    statement = ''
+
+class StormStop(StormGenrCtrl, StormCtrlFlow):
+    statement = 'stop'
+
+class StormBreak(StormLoopCtrl, StormCtrlFlow):
+    statement = 'break'
+
+class StormContinue(StormLoopCtrl, StormCtrlFlow):
+    statement = 'continue'
+
+class StormExit(_SynErrMixin, StormCtrlFlow): pass
+
+# StormReturn is kept thin since it is commonly used and just
+# needs to be the container for moving an item up a frame.
+class StormReturn(StormCtrlFlow):
     def __init__(self, item=None):
         self.item = item
-
-class StormExit(StormCtrlFlow): pass
-class StormStop(StormCtrlFlow): pass
-class StormBreak(StormCtrlFlow): pass
-class StormReturn(StormCtrlFlow): pass
-class StormContinue(StormCtrlFlow): pass

synapse/lib/stormlib/cache.py

@@ -173,6 +173,7 @@ async def _runCallback(self, key):
             except s_stormctrl.StormReturn as e:
                 return await s_stormtypes.toprim(e.item)
             except s_stormctrl.StormCtrlFlow:
+                # DISCUSS / FIXME: This should probably convert the CtrlFlow into a StormRuntimeError?
                 pass
 
     async def _reqKey(self, key):

synapse/lib/stormtypes.py

@@ -1663,7 +1663,7 @@ async def _exit(self, mesg=None, **kwargs):
         if mesg:
             mesg = await self._get_mesg(mesg, **kwargs)
             await self.runt.warn(mesg, log=False)
-            raise s_stormctrl.StormExit(mesg)
+            raise s_stormctrl.StormExit(mesg=mesg)
         raise s_stormctrl.StormExit()
 
     @stormfunc(readonly=True)

synapse/lib/view.py

@@ -635,7 +635,6 @@ def isForkOf(self, viewiden):
     async def _calcForkLayers(self):
         # recompute the proper set of layers for a forked view
         # (this may only be called from within a nexus handler)
-
         '''
         We spent a lot of time thinking/talking about this so some hefty
         comments are in order:
@@ -953,6 +952,15 @@ async def callStorm(self, text, opts=None):
                            extra={'synapse': {'text': text, 'username': user.name, 'user': user.iden}})
             raise
 
+        except (s_stormctrl.StormLoopCtrl, s_stormctrl.StormGenrCtrl) as e:
+            if isinstance(e, s_stormctrl.StormLoopCtrl):
+                mesg = f'Loop control statement "{e.statement}" used outside of a loop.'
+            else:
+                mesg = f'Generator control statement "{e.statement}" used outside of a generator function.'
+            logmesg = f'Error during storm execution for {{ {text} }} - {mesg}'
+            logger.exception(logmesg, extra={'synapse': {'text': text, 'username': user.name, 'user': user.iden}})
+            raise s_exc.StormRuntimeError(mesg=mesg, statement=e.statement, highlight=e.get('highlight')) from e
+
         except Exception:
             logger.exception(f'Error during callStorm execution for {{ {text} }}',
                              extra={'synapse': {'text': text, 'username': user.name, 'user': user.iden}})
@@ -1055,8 +1063,17 @@ async def runStorm():
                 raise
 
             except Exception as e:
-                logger.exception(f'Error during storm execution for {{ {text} }}',
-                                 extra={'synapse': {'text': text, 'username': user.name, 'user': user.iden}})
+                mesg = ''
+                if isinstance(e, s_stormctrl.StormLoopCtrl):
+                    mesg = f'Loop control statement "{e.statement}" used outside of a loop.'
+                    e = s_exc.StormRuntimeError(mesg=mesg, statement=e.statement, highlight=e.get('highlight'))
+                elif isinstance(e, s_stormctrl.StormGenrCtrl):
+                    mesg = f'Generator control statement "{e.statement}" used outside of a generator function.'
+                    e = s_exc.StormRuntimeError(mesg=mesg, statement=e.statement, highlight=e.get('highlight'))
+                logmesg = f'Error during storm execution for {{ {text} }}'
+                if mesg:
+                    logmesg = f'{logmesg} - {mesg}'
+                logger.exception(logmesg, extra={'synapse': {'text': text, 'username': user.name, 'user': user.iden}})
                 enfo = s_common.err(e)
                 enfo[1].pop('esrc', None)
                 enfo[1].pop('ename', None)