search

breadboard-ai / breadboard

A library for prototyping generative AI applications.
Apache License 2.0
172 stars 23 forks source link

Auto-save can lead to irrecoverable states #3206

Open dglazkov opened 1 week ago

dglazkov commented 1 week ago

Just ran into this: I pasted the URL of what looks like a BGL file, but actually wasn't. Allowing this is likely a bug in itself. But what happened then is that the board stopped rendering, since it contains a node with an invalid type. Then it auto-saved and I panicked and refreshed. This removed the undo history, and now I had an blank editor and now way to recover.

paullewis commented 1 week ago

As of https://github.com/breadboard-ai/breadboard/commit/fd69479402ef2627fb5d47354d55adcfb4abd79c we now detect a failure to obtain the type data for a node, and we a) continue to show the graph, and b) show a toast error explaining that we couldn't get the type info (and suggest removing the node).

The main change was to add a try-catch here https://github.com/breadboard-ai/breadboard/blob/main/packages/shared-ui/src/elements/editor/editor.ts#L523-L531 since we assumed that the type retrieval would be successful.

That said, I've not marked this as fixed because I wonder if we want to consider a couple of additional options:

  1. At the lowest level we have an internal handler for the CustomNodeType that can throw (and which we don't currently handle) – it feels like we could add error handling here so that we land safely in the event that we can't load the node type. (It feels like this could also happen if we're offline and attempt to load a board from a board server.)
  2. When we go to add a custom node type we could try and fetch the URL before adding the node to the board. This would catch copy-paste errors, but wouldn't solve the above case of being offline after adding a node.
paullewis commented 1 week ago

Pondering a little more and thinking we might want some kind of status on the InspectableNode that indicates there was an issue (as well as catching the failed fetch) so we can show something in the graph maybe?

dglazkov commented 1 week ago

Yeah something like that! I was also thinking about that maybe right at drop time, I could give you a "IS THIS EVEN A NODE TYPE?" checker. We're going to immediately try loading this graph anyway, so might as well do it early. WDYT?

paullewis commented 1 week ago

Yeah, I think both... a sort-of "okay, this seems valid" API is a good first pass, and then something on the InspectableNode in case we fail to resolve it later because you're offline or because the board no longer exists?