Multiserver, Docs: Datastorage error handling + docs

[Jarno458]

trafficstars

What is this fixing or adding?

• An on_error argument to datastorage Set operations, that allow the implementer to specify how errors should be handed
• Docs on how to work with the datastorage in a thread safe way

How was this tested?

unit tests + a few manual full stack tests with running a local server also tested generating on main, adding data to the datastorage, and then loading the saved multiworld on this branch to see if the data would transfer over to the new data structure

[black-sliver]

Sorry for the super late reply :S

I think you missed what I was suggesting: instead of verifying everything first, you simply try to run it. That way the "happy path" is free. You only need a verify the bits where that doesn't work - i.e. the things where "ignore" would apply, but shouldn't. I think only if set_cmd["key"].startswith("_read_"): needs explicit verification if you refactor your for operation in .... If you'd keep that loop as-is, the operations verification could at least be collapsed into a single isinstance, since if operations is missing, that will raise anyway.

For example for if "key" not in set_cmd:: when you try to run value = self.stored_data.get(set_cmd["key"], set_cmd.get("default", 0)) and key is missing, you already get an exception for that, so there is no need to verify "key" in set_cmd.

[Jarno458]

Oke, but i will need some upfront validation to be able to distinct between an invallid request, resulting in an InvalidPacket response, or an exception during processing that can be handled by on_error

original check on the server to return an InvallidPacket was:

 if "key" not in args or args["key"].startswith("_read_") or \
                    "operations" not in args or not type(args["operations"]) == list:

so i want to atleast preserve this to not cause breaking changes

[black-sliver]

Hm, tests are failing.

Also you did not use the Any at all, please read the text in the suggestion again.

[black-sliver]

As said on discord, I think this is fine now, but I would really like to see approval from world authors or other potential users of the changed API.

[Jarno458]

Understandable, i tried to poke a few potentional reviewes, didnt get any results yet

[Jarno458]

Note to self: If #3747 is merged, this will need an update