Error code bug in C++ when deleting an item

[Kanellaman]

After deleting successfully an item from keychain an error code is raised resulting in exception in C# code.

I run on this problem while testing the C# code on Windows.

Program.cs

using System;
namespace KeySharp
{
    internal class Program
    {
        static void Main(string[] args)
        {
            Keyring.SetPassword("Lalala", "Test", "thisuser", "Neone");
            try
            {
                var password = Keyring.GetPassword("Lalala", "Test", "thisuser");
                Console.WriteLine(password);
            }
            catch (KeyringException ex)
            {
                Console.WriteLine(ex.Message);
            }
            Keyring.DeletePassword("Lalala", "Test", "thisuser");
        }
    }
}

Output:

Neone

Unhandled Exception: KeySharp.KeyringException:  (NoError)
   at KeySharp.Keyring.ThrowLastError() in C:\Users\user\Source\Repos\KeySharp\KeySharp\Keyring.cs:line 114
   at KeySharp.Keyring.DeletePassword(String package, String service, String username) in C:\Users\user\Source\Repos\KeySharp\KeySharp\Keyring.cs:line 84
   at KeySharp.Program.Main(String[] args) in C:\Users\user\Source\Repos\KeySharp\KeySharp\Program.cs:line 18

The fix I am suggesting is changing the 50th line of main.cpp to if (error->type != keychain::ErrorType::NoError) similar to other functions.
I have not actually tested the change I am suggesting but I am pretty sure this is where the error occurs(I'll try to re-compile the C++ code to test it). It would be awesome if you could verify.

[OmarResponsival]

It would be awesome if this got fixed, I currently have to do a try catch and check the error type which it works but is throwing my code coverage on testing a little off. I rather not fork since I really wouldn't be adding any functionality to it, the library works great except for this small thing.

[goaaats]

I'd be very happy to accept a pull request with this fix and rebuild the natives/release a new version! I sadly don't have time to look into it myself at the moment though.

[Kanellaman]

I'd be very happy to accept a pull request with this fix and rebuild the natives/release a new version! I sadly don't have time to look into it myself at the moment though.

Next week I will look into it and have a pull request made. No worries.

[OmarForever]

I'd be very happy to accept a pull request with this fix and rebuild the natives/release a new version! I sadly don't have time to look into it myself at the moment though.

Next week I will look into it and have a pull request made. No worries.

I opened a pull request with your fix, I tested locally and works properly.

[Kanellaman]

Mind if you accept the pull request and close the issue as resolved? @goaaats

[goaaats]

I merged the pull request, thanks Omar! I probably won't have time to make releases for a little while, so I'll leave the ticket open till then and update it when I get around to it.

[vedantmgoyal9]

@goaaats please create a new release

[vedantmgoyal9]

@goaaats https://github.com/hrantzsch/keychain/releases - v1.3.0 migrates to new keychain API for macOS. I've created #4 which adds dependabot for automatically updating submodule, and a CI to update pre-compiled libraries as well. The library is currently in an unusable state until the pre-compiled libraries have been updated. I request you to merge the PR and create a new release as soon as possible 🙏🏻

[vedantmgoyal9]

@goaaats hello?