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.

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

feat(ui): canvas error handling #6896

Merged
merged 5 commits into from
Sep 20, 2024
Merged

Conversation

psychedelicious
Copy link
Collaborator

Summary

I reviewed the canvas codebase to find places where errors could occur. Two categories:

  • Network requests (e.g. getting images, uploading images, queuing graphs)
  • Canvas operations (e.g. exporting canvas to Blob)

All instances of these things now have error handling at some level - either close to the error-producing code or higher up where the code is consumed.

Canvas filters were the most complex thing to handle. The filter queues a graph, waits for a specific node to finish and retrieves an output image. Handling all the possible cases requires care. There's a new method that wraps up executing a graph to get an image output into a single async function call. This is well-documented.

While iterating on filters, I made an internal change to how filter graphs are generated. Instead defining a node, filters define a graph and a node that outputs an image. This makes filters very flexible - so long as the graph has a node that outputs an image, it can be run as a filter.

See commit messages and code for details.

Related Issues / Discussions

Discord & offline discussion

QA Instructions

Hit the things that changed:

  • Run some filters.
  • Run a long-running filter (e.g. depth anything on a huge layer) and cancel it.
  • Do some transforms (has improved error handling).
  • Run a graph w/ a controlnet, t2i adapter and regional guidance (these now have error handling).

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)

- Rename util to `getImageDTOSafe`
- Update API to accept the same options as RTKQ's `initiate`
- Add `getImageDTO`; while `getImageDTOSafe` returns null if the image is not found, the new util throws
- Update usage of `getImageDTOSafe`
Two main changes:
- Add `runGraphAndReturnImageOutput` to `CanvasStateApiModule`. This method is a safe and convenient abstraction to execute a graph and retrieve the image output of one of its nodes. It supports cancellation (via an AbortSignal) and timeout.
- Update filters to build whole graphs, as opposed to nodes.

These changes allow:
- Filter execution is resilient, with all error cases handled (afaik)
- `CanvasEntityFilterer` class is much simpler
- Stuck or long-running filters may be canceled
- Filters may be arbitrarily complex - so long as there is one node that outputs an image, the filter will just work
@github-actions github-actions bot added the frontend PRs that change frontend files label Sep 20, 2024
@hipsterusername hipsterusername merged commit 7ddbdd5 into main Sep 20, 2024
14 checks passed
@hipsterusername hipsterusername deleted the psyche/canvas-error-handling branch September 20, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend PRs that change frontend files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants