Skip to content

Conversation

julianstirling
Copy link
Contributor

@julianstirling julianstirling commented Jul 27, 2025

This needs testing before merge

This fixes two related things:

1. Events being cancelled by the user are logged as errors.

image

While error handling is used for cancelling the event. From the user's perspective no error happened. They asked for it to be cancelled, and it was. This PR changes that log to "Info", so that the event is logged, but the user isn't notified of an error for expected behaviour.

2. Adds a new exception type without a traceback.

Currently any unhandled errors are shown with an ASGI traceback. Very useful for developers, but not friendly for the end user. These make sense for unknown errors.

In the OpenFlexure Microscope we handle this by catching errors, logging the error and then exiting. This does not notify as an error on exit correctly unless they go to the log. However a full ASGI error traceback doesn't communicate the message needed.

This should create an error type that ends the invocation, logs without traceback, but creates the correct error response when the invocation completes.

Hmm, no they are not always ASGI errors. But, it still seems like a good idea for an action to be able to communicate "I handled this error, but I am stopping" and for this to be treated differently to an unhandled exception in an action.

Copy link

barecheck bot commented Jul 27, 2025

Barecheck - Code coverage report

Total: 93.31%

Your code coverage diff: 0.01% ▴

Uncovered files and lines
FileLines
src/labthings_fastapi/actions/__init__.py145-146, 412-413, 534-535, 540, 548, 573-574
src/labthings_fastapi/dependencies/invocation.py161-162

@julianstirling julianstirling force-pushed the cleaner-logs-for-errors branch from 1be3dfe to fbad976 Compare August 13, 2025 12:31
@julianstirling julianstirling marked this pull request as ready for review August 13, 2025 12:31
@julianstirling julianstirling changed the title Add InvocationError for an error without traceback. Alog cancel at info. Add InvocationError for an error without traceback. Log cancel at info. Aug 13, 2025
@julianstirling julianstirling force-pushed the cleaner-logs-for-errors branch from c5be363 to b171f44 Compare August 13, 2025 14:05
Copy link
Collaborator

@rwb27 rwb27 left a comment

Choose a reason for hiding this comment

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

This looks good to me.

We need to standardise whether exceptions get re-exported from .exceptions or whether they are defined there and imported where they're needed. The latter would play better with the API docs, but we should standardise this in its own PR.

@rwb27 rwb27 merged commit 439989d into main Aug 13, 2025
14 checks passed
@rwb27 rwb27 deleted the cleaner-logs-for-errors branch August 13, 2025 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants