Detect and display non-zero returncode from scripts

[will-moore]

This uses the returncode returned from running scripts to show the Error icon in Activities dialog:

Feature requested at https://forum.image.sc/t/omero-scripts-fail-icon/81495

To test:

• Run a script that throws an Exception after setting message, e.g. client.setOutput("Message", rstring("OK so far..."))
  • The Message will be shown, along with Error status (first result below)
• Run a script that throws an Exception without setting "Message" (typical behaviour)
  • This will display a message including the returncode from the failure.
• Otherwise working scripts should behave as before (2 other examples below)

The context used to render the html page in the example above looks like this:

{
  "sizeOfJobs": 4,
  "jobs": [
    {
      "job_type": "script",
      "job_name": "Images From ROIs",
      "start_time": "datetime.datetime(2023, 10, 18, 10, 15, 20, 121058)",
      "status": "finished",
      "returncode": 1,
      "Message": "OK so far...",
      "stderr": 3725,
      "results": {},
      "id": "52dd4857-9ca1-4dd1-ad0b-5a78d3fdd235",
      "key": "ProcessCallback/52dd4857-9ca1-4dd1-ad0b-5a78d3fdd235 -t -e 1.1:tcp -h 172.19.0.4 -p 37387 -t 60000",
      "new": true
    },
    {
      "job_type": "script",
      "job_name": "Dataset To Plate",
      "start_time": "datetime.datetime(2023, 10, 18, 10, 16, 0, 665853)",
      "status": "finished",
      "returncode": 1,
      "Message": "Script exited with failure. (returncode=1)",
      "stderr": 3724,
      "results": {},
      "id": "087c20ed-0617-4931-9ef4-91046bd1181e",
      "key": "ProcessCallback/087c20ed-0617-4931-9ef4-91046bd1181e -t -e 1.1:tcp -h 172.19.0.4 -p 37387 -t 60000"
    },
    {
      "job_type": "script",
      "job_name": "Dataset To Plate",
      "start_time": "datetime.datetime(2023, 10, 18, 10, 15, 20, 121058)",
      "status": "finished",
      "returncode": 0,
      "Message": " New plate created: From_ROIs but could not be attached.",
      "results": {
        "New_Object": {
          "id": 101,
          "type": "Plate",
          "browse_url": "/webclient/userdata/?show=plate-101",
          "name": "From_ROIs"
        }
      },
      "id": "e31c309c-947c-4fa7-a8ff-1471f31a52d1",
      "key": "ProcessCallback/e31c309c-947c-4fa7-a8ff-1471f31a52d1 -t -e 1.1:tcp -h 172.19.0.4 -p 37387 -t 60000"
    },
    {
      "job_type": "script",
      "job_name": "Channel Offsets",
      "start_time": "datetime.datetime(2023, 10, 18, 10, 10, 54, 262510)",
      "status": "finished",
      "returncode": 0,
      "Message": "New image created: P-TRE_10_R3D_D3D.dv_0_offsets.",
      "stdout": 3719,
      "results": {
        "Image": {
          "id": 751,
          "type": "Image",
          "browse_url": "/webclient/userdata/?show=image-751",
          "name": "P-TRE_10_R3D_D3D.dv_0_offsets"
        }
      },
      "id": "6447e666-fc04-42ff-955c-d1cfe2e23035",
      "key": "ProcessCallback/6447e666-fc04-42ff-955c-d1cfe2e23035 -t -e 1.1:tcp -h 172.19.0.4 -p 37387 -t 60000"
    }
  ],
  "inprogress": 0,
  "new_results": 1,
  "new_errors": true,
  "failure": 0
}

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/omero-scripts-fail-icon/81495/7

[sbesson]

Interesting thread which incidentally raises the question: what the recommendation should be for handling and communicating failures in OMERO.scripts?

At the moment, the primary relevant place I found in the reference documentation suggests failures may be handled via client output message - see https://omero.readthedocs.io/en/stable/developers/scripts/style-guide.html#script-outputs

Using the official Populate_Metadata.py OMERO script as an exmple, there is already a mixture of behaviours:

• some failing checks write to sys.stderr and raisesys.exit(1) - e.g. https://github.com/ome/omero-scripts/blob/88219a3323685a575f92e689222b91b4e590da3c/omero/import_scripts/Populate_Metadata.py#L65-L68. These would be displayed with an error icon as per the current state of this PR
• other locations return an error message which is sent back to the client as per the reference guide - see https://github.com/ome/omero-scripts/blob/88219a3323685a575f92e689222b91b4e590da3c/omero/import_scripts/Populate_Metadata.py#L112-L115

[will-moore]

@sbesson Thanks for the review. I guess we can't currently have the script raise an Exception (to get the failed icon) and also return a useful Error message. But the Exception will be captured in stdout - do we need to make it more obvious that the user should look there?

I would prefer not to add another convention (on top of the "Message" convention) for errors, so I think that raising an Exception should be the best way to indicate "Failure".

[will-moore]

That last commit fixed the display of Error reporting in Figure:

[sbesson]

@will-moore following up on the questions from https://github.com/ome/omero-web/pull/474#discussion_r1219073402, would it be possible to either update the description of this PR or add a comment with the different types of scenarios that should be tested and the expected outcome of the callback dictionary ?

[will-moore]

@sbesson I've updated the PR description. Hope that contains everything you need?

[sbesson]

Thanks @will-moore for adjusting the description. As per the previous discussion, I am still questioning the added value of the new failure key as its value seems to be completely redundant with the presence/value of error. I thought the plan was to remove it as per https://github.com/ome/omero-web/pull/474#discussion_r1321661476 ?

Also re https://github.com/ome/omero-web/pull/474#discussion_r1203972716, has the proposal of storing the value of the return code in error and adjust the HTML template to handle the non-zero scenario been discussed further?

[will-moore]

OK - yes, apologies. I'll do that...

[will-moore]

@sbesson Use key returncode now for the returncode! Updated the JSON in the description accordingly.