more robust validation of return value from destfnscript

[petersilva]

saw how inscrutable the error message was for Daniel, when his destfnscript had a problem.
wrapped it in a try/except and added minimal validation of the return value... give people a hint.

[github-actions]

Test Results

242 tests   234 ✅  19s ⏱️
  1 suites    8 💤
  1 files      0 ❌

Results for commit 7efabbf.

♻️ This comment has been updated with latest results.

[petersilva]

here is the old "error message":

Traceback (most recent call last):
  File "/local/home/sarra/sr3/sarracenia/instance.py", line 248, in <module> i.start()
  File "/local/home/sarra/sr3/sarracenia/instance.py", line 239, in start self.running_instance.run()
  File "/local/home/sarra/sr3/sarracenia/flow/__init__.py", line 565, in run  self.filter()
  File "/local/home/sarra/sr3/sarracenia/flow/__init__.py", line 1020, in filter pstrip, flatten)
  File "/local/home/sarra/sr3/sarracenia/flow/__init__.py", line 928, in updateFieldsAccepted
    msg['new_file'] = self.sundew_getDestInfos(msg, maskFileOption, filename)
  File "/local/home/sarra/sr3/sarracenia/flow/__init__.py", line 746, in sundew_getDestInfos
    return destFileName + satnet + timeSuffix
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

and the instance crashes.

[reidsunderland]

There's a change in behaviour. Before the instance would crash and the file wouldn't get sent. Now if the destfnscript crashes, the file gets sent with the original name.

Is that okay? Would it be better to reject the message?

[petersilva]

I dunno what is best to do... if it crashed then it would be obvious that something is wrong... with a destfn script... I mean it will drop some files in the flow... I guess missing is easier to understand than named wrong?

ok.

[petersilva]

OK... so the problem is that we aren't in mainline flow code... we are in the routine sundew_getDestInfos whose return value is the new file name... and that is called from updateFieldsAccepted... which has no return value...
how to signal, the message is broken... maybe setting an error field in the message?

[reidsunderland]

That could work.

I'm not saying this is better, just an alternative option: sundew_getDestInfos is called from updateFieldsAccepted, and that is called in the filter method. The try/except could be moved into filter, and the messages could be rejected there?

[petersilva]

somehow this is already in development branch... I'm confused... merged by some other merge... anyways I will try to make it return None, and have the various callers deal with that to do something smart.

[petersilva]

OK... it returns None as the file name in the error case, and now the main routines do_download and do_send check for empty new_file and new_directory fields and reject them. Good?

[reidsunderland]

yes, that's great. thanks