Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect and display non-zero returncode from scripts #474

Merged
merged 10 commits into from
Oct 19, 2023
Merged

Detect and display non-zero returncode from scripts #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
803f18c
Detect and display non-zero returncode from scripts
will-moore May 24, 2023
bd9f57d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 24, 2023
f5384a2
Add returncode to error message
will-moore May 24, 2023
f4c0fcd
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 24, 2023
1879956
flake8 fix
will-moore May 24, 2023
8081fa7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 24, 2023
c9a586d
Revert Comment change to avoid merge conflict
will-moore May 29, 2023
c88fdf7
Merge remote-tracking branch 'origin/master' into script_failure_hand…
will-moore May 31, 2023
659d4c5
Always use status:'finished' when done, even if error
will-moore Oct 10, 2023
760da8e
Use 'returncode' instead of 'failure' and 'error'
will-moore Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions omeroweb/webclient/templates/webclient/activities/activitiesContent.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,22 +240,9 @@
<img alt="Running Script" src="{% static "webgateway/img/spinner.gif" %}" />

{% else %}
{% if j.error %}
<div class='script_error' title="{{ j.error }}">
{% if j.returncode and j.returncode > 0 %}
<div class='script_error' title="Script failed. Please see error logs">
<img alt="Failed to run script properly" src="{% static "webgateway/img/failed.png" %}" />


{% comment %}
<!-- Don't allow submitting now. TODO: launch submit page -->
<img src="{% static "webclient/image/info16.png" %}"/>
<input type='submit' title="Send Error as Feeback to the OME team" jobKey="{{ j.key }}"
{% if j.error_sent %}
value='Thank you' disabled='true'
{% else %}
value='Submit Feedback'
{% endif %} />
{% endcomment %}

</div>
{% else %}
<img alt="Success" src="{% static "webgateway/img/success.png" %}" />
Expand Down
20 changes: 18 additions & 2 deletions omeroweb/webclient/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3524,6 +3524,13 @@ def update_callback(request, cbString, **kwargs):
request.session["callback"][cbString][key] = value


# Subclass to handle returncode
class ScriptsCallback(omero.scripts.ProcessCallbackI):
def processFinished(self, returncode, current=None):
super().processFinished(returncode, current)
self.returncode = returncode


@login_required()
@render_response()
def activities(request, conn=None, **kwargs):
Expand Down Expand Up @@ -3799,14 +3806,23 @@ def activities(request, conn=None, **kwargs):
error=1,
)
continue
cb = omero.scripts.ProcessCallbackI(conn.c, proc)
cb = ScriptsCallback(conn.c, proc)
# check if we get something back from the handle...
if cb.block(0): # ms.
cb.close()
try:
# we can only retrieve this ONCE - must save results
results = proc.getResults(0, conn.SERVICE_OPTS)
update_callback(request, cbString, status="finished")
kwargs = {
"status": "finished",
"returncode": cb.returncode,
}
if cb.returncode != 0:
kwargs["Message"] = (
f"Script exited with failure."
f" (returncode={ cb.returncode })"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen if a script first set a message output via client.setOutput and then terminate with a non-zero return code?
Should we handle this use case and respect the message sent by the script and use this as the default error is unset?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbesson Just returning to this... In my testing, if the script does client.setOutput(....) then generates some un-caught exception and terminates, the output is not passed on to the server. I don't see it in the code at

omero-web/omeroweb/webclient/views.py

Line 3822 in d5e4f84

for key, value in results.items():

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies - made a mistake in my client.setOutput("Message"...). That is actually handled OK and is displayed in place of the error message:

Screenshot 2023-09-08 at 14 37 37

However, most scripts don't try to return a "Message" until the end, so it's likely that if a script fails with some uncaught exception then the "Message" won't have been set.

update_callback(request, cbString, **kwargs)
new_results.append(cbString)
except Exception:
update_callback(
Expand Down
Loading