feat: use serde_path_to_error for better parsing errors

[jonas-w]

The parsing errors now use the serde_path_to_error library as I proposed in #11

I'm not too sure on two things:

1. I didn't need (i.e. I couldn't get it working with it, and wasn't too sure what it's use case was with serde) the UntaggedEnumHelper anymore, maybe you can take a look at this if this is correct.

2. I moved printing the response from the error! statement to a debug! statement for two reasons:

• serde_path_to_error is already descriptive enough: e.g. for the issue that I fixed in fix: module indent can be negative #11 It would look like this:

Could not deserialize response err=Error { path: Path { segments: [Seq { index: 6 }, Map { key: "modules" }, Seq { index: 7 }, Map { key: "indent" }] }, original: Error("invalid value: integer `-1`, expected u64", line: 1, column: 35913) }

• And the whole response may be rather large and isn't even helpful to the average user, which may even hide the error, if too many things fail and the terminall scrollback history is too small.

This fixes #14

[jonas-w]

FWIW cargo also told me that thiserror has a v2 available (edu-sync/Cargo.toml currently specifies 1.0), edu-sync compiles and tests are passing with it bumped. This could be also added to the next release

[mkroening]

1. I didn't need (i.e. I couldn't get it working with it, and wasn't too sure what it's use case was with serde) the UntaggedEnumHelper anymore, maybe you can take a look at this if this is correct.

That one was relatively important. There are different kinds of errors that this function may return. There might be a parsing error that we want to improve the error message for, but there might also be a server-side error, for example if we provide an invalid token. The UntaggedResultHelper was there to make sure that first we try to deserialize the successful message from the server and if that fails to try to deserialize a server-side error message. The result was this very unhelpful "data did not match any variant of untagged enum Result" error message in case both variants could not be deserialized.

To make sure that server-side error messages are also properly parsed, I have amended your commit to manually try to deserialize errors. It performs to my expectations in my tests. Does this look good to you?

2. I moved printing the response from the error! statement to a debug! statement

That's good! 👍

[mkroening]

Thanks!

[jonas-w]

No worries! Are you going to update the AUR packages once you've created a new release? (I saw that you are the maintainer on the AUR)

[mkroening]

Are you going to update the AUR packages once you've created a new release? (I saw that you are the maintainer on the AUR)

Good question. I have also thought about that recently. I am not using Arch myself anymore, that's why I don't really maintain that package anymore. I was considering removing it from the README and orphaning the package. Are you interested in maintaining the package?

I could, of course, bump the versions myself occasionally, but without proper testing or using the package I am not sure if that is beneficial.

[jonas-w]

@mkroening Sure, why not! I think it's also possible to add co maintainers, this way I wouldn't be solely responsible, and you could still have the power over the package. I couldn't really find how to do this, but on the AUR frontpage it says "Co-Maintained Packages" which currently are none, so it should be somehow possible.

Okay I think It's on the Package Page once you're logged in there is a button called 'Manage Co-Maintainers' in the 'Package Actions' section.

[mkroening]

Awesome! I have just published version 0.2.3 on GitHub and added you as co-maintainer for both packages in the AUR. :)

[jonas-w]

@mkroening Perfect! I suppose the dbus dependency isn't needed anymore? At least I can't find any mentions of it in edu-sync anymore? If I saw it correctly, this was used for keyring functionallity, which was removed and the token stored into the config

[mkroening]

Yes, that's correct. 👍

[jonas-w]

I hoped it would be just a quick version bump, but somehow it doesn't compile the PKGBUILD and fails with 'undefined symbolregardingring_core` :(

Compiling v0.2.3 manually works completely fine, but somehow not through makepkg via the PKGBUILD.

Don't know if this is an issue with my setup, will need to look into this tomorrow.

[jonas-w]

@mkroening Okay, don't know what the core issue really is, but it's related to briansmith/ring#1444
adding !lto to the options array solved it options=(!strip !lto).

Some other things I noticed (while debugging the linking error), the edu-sync crate can't be compiled standalone, as it doesn't specify macros as a feature for tokio
changing the line in the edu-sync/Cargo.toml to the following makes it work again:
tokio = { version = "1", default-features = false, features = ["macros","fs"] }

I also noticed that some modules are specified in different crates with different feature flags, especially reqwest and tokio. I'm not that proficient in rust, but if I understand it correctly, this increases the compile time, as it needs to compile tokio and reqwest 4 times.

Both are not blocking me from updating the AUR packages, just wanted to mention it.
If makepkg does not make any further problems, I'll bump the AUR packages.

Done, have updated the versions, and at least it installed with paru on my system