Ockam CLI should gracefully handle invalid state when initializing

[SanjoDeundiak]

Observed behavior

Currently, ockam_command expects its state to be valid, and if it's not - it panics here

Steps to reproduce

1. Remove ockam state directory (usually it's ~/.ockam)
2. Run ockam identity create
3. Go to ~/.ockam/identities/{ID}.json file and mangle it (e.g., change the json key value from identifier to id)
4. Run ockam reset --yes

Expected behavior

1. At least for ockam reset command, we should ignore this error and wipe the state so that the next command would start from scratch and succeed.
2. We should handle this error gracefully and return a proper error instead of panicking

[0xphen]

@SanjoDeundiak I'm thinking the new error should be added here. What should be the name of the error ?

Or do I make use of an existing error (like InternalError) or a CliStateError?

[etorreborre]

Hi @0xphen this error is more of a CliStateError since it concerns the persistence of data by the ockam_api crate. This is:

• a 500 error
• with a type like IncorrectFileFormat (so that it can be skipped by ockam reset -y)
• and a help text saying that users need to reset their command line state by running ockam reset -y (if another command led to a broken state)

[DogPawHat]

Was thinking of taking this if it available.

[etorreborre]

Hi @DogPawHat, that's very kind of you. Let's check with @0xphen if they have started working towards this or not.

[davide-baldo]

@DogPawHat

It seems like the issue can be taken! Are you still interested in this development?

[DogPawHat]

Yeah. It seems like I'd have to maybe separate out the directory and file parts of CliState apart, as you have a chicken and egg situation where you need a CliState to reset the .ockam directory, you can't get a CliState if you have an error, so you need to reset the directory...

Would appreciate advise and help on this.

[0xphen]

@davide-baldo @etorreborre apologies for taking too long to respond (I haven't been available for the past 2 weeks). @DogPawHat I'm glad you're working on this; here's what I have gained so far from this issue:

The error originates when trying to initialize the IdentitiesState. It tries to migrate the configuration and fails. But from looking through the code, the ockam reset -y command doesn't need or depend on the configuration being migrated; since it resets the configuration.

[DogPawHat]

Can we have this assigned to me if we're all agreed?

[DogPawHat]

@0xphen thanks for your write up, that mostly tracks with what I've seen in the code.

[davide-baldo]

Thanks to both of you, It's wonderful to see this community grow and collaborate ❤️

[DogPawHat]

Small update, I'm still hacking away at this. I was trying to separate StateItemTrait into separate state traits (a la state pattern), in that the pre-validation state had just the path of the file, while the validated state had the de serialized config, the idea being to defer the de-serialization to the transition method (implemented as TryFrom) so that we could still used the delete method on the unvalidated file. Unfortunately I was soon adding extra method called to everywhere that needed the Item .config(), which was most of the codebase that did need the validated state.

I've changed tack to attaching a struct to the error produced when serde fails to parse so that the error handling logic can call .delete() in case ockam reset is called. This should be achievable with less LOC and a smaller PR.

[DogPawHat]

Hi, @adrianbenavides, I've a PR in draft, but I think your PR at #6071 has this issue solved now, although I think it resets with any command if CliState fails to initialize.

[adrianbenavides]

@DogPawHat yes, my fault. We had to push this forward as it was impacting our production builds. I didn't see this issue when I created that PR and I never updated it.

I'm closing the issue, but let me know if I can help you with any other issue you might be interested in, we have a lot of new issues that we created for the Hacktoberfest.

Thank you so much for spending your time on this 🙏 I hope you can find another issue to contribute to Ockam!