Language-ext: OptionAsync impicit cast throws error when value is null

Created on 19 Nov 2020  路  5Comments  路  Source: louthy/language-ext

I am getting exception with 3.5.20-beta while trying to use OptionAsync as a wrapper over nullable result

public class Test
    {
        public string value => "test";
    }
    class Program
    {
        static async Task Main(string[] args)
        {
            await test();
            Console.ReadKey();
        }

        private static async Task test()
        {
            Console.WriteLine("Test started");
            Console.WriteLine("Wrap for some: {0}", (await wrap(true))?.value ?? "ERROR");
            Console.WriteLine("Wrap for none: {0}", (await wrap(false))?.value ?? "Empty");  // throws

            Console.WriteLine("Test ended"); 
        }
        private static OptionAsync<Test> wrap(bool some)
        {
            return some ? delaySome() : delayNone();
        }

        private static async Task<Test> delaySome()
        {
            await Task.Delay(2000);
            return new Test();
        }

        private static async Task<Test> delayNone()
        {
            await Task.Delay(2000);
            return null;
        }

BUG:
Test started
Wrap for some: test
Unhandled exception. LanguageExt.ValueIsNoneException: Value is none.
at LanguageExt.OptionAsyncAwaiter`1.GetResult()
at LanguageExt_OptionAsync.Program.test() in D:ProjectsLanguageExt_OptionAsyncLanguageExt_OptionAsyncProgram.cs:line 23
at LanguageExt_OptionAsync.Program.Main(String[] args) in D:ProjectsLanguageExt_OptionAsyncLanguageExt_OptionAsyncProgram.cs:line 15
at LanguageExt_OptionAsync.Program.

(String[] args)

Most helpful comment

This is because the implicit converter for Task<A>....
https://github.com/louthy/language-ext/blob/3799d385a88ab100af7f64e21eb154101db4c338/LanguageExt.Core/DataTypes/OptionAsync/OptionAsync.cs#L148-L149
...calls this constructor that expect the value in the Task<> to be not be null.
https://github.com/louthy/language-ext/blob/3799d385a88ab100af7f64e21eb154101db4c338/LanguageExt.Core/DataTypes/OptionAsync/OptionAsync.cs#L85-L86

To match the behavior for Option<>, I do think an instance of OptionAsync<> should be returned in the None state. Do you agree @louthy?

P.S.
Some of the documentation seem a bit wrong to me. I looks like it was copy-pasted from Option<> and between methods without all the needed modifications.

All 5 comments

This is because the implicit converter for Task<A>....
https://github.com/louthy/language-ext/blob/3799d385a88ab100af7f64e21eb154101db4c338/LanguageExt.Core/DataTypes/OptionAsync/OptionAsync.cs#L148-L149
...calls this constructor that expect the value in the Task<> to be not be null.
https://github.com/louthy/language-ext/blob/3799d385a88ab100af7f64e21eb154101db4c338/LanguageExt.Core/DataTypes/OptionAsync/OptionAsync.cs#L85-L86

To match the behavior for Option<>, I do think an instance of OptionAsync<> should be returned in the None state. Do you agree @louthy?

P.S.
Some of the documentation seem a bit wrong to me. I looks like it was copy-pasted from Option<> and between methods without all the needed modifications.

@TysonMN @louthy is there any update?

To match the behavior for Option<>, I do think an instance of OptionAsync<> should be returned in the None state. Do you agree @louthy?

I do. I will accept a PR for this.

@Lonli-Lokli, do you think you could contribute a PR that implements your desired behavior?

@TysonMN I am trying to build, seems like DevPack for .NET Framework 4.6.1 is required, not sure I will be able to install it over existing one.

But nevermind, I will be able to send PR.
EDIT: send.

I also noticed that async XUnit tests are not correct:

now:

  [Fact]
  public async void InitialTests()

should be:

  [Fact]
  public async Task InitialTests()
Was this page helpful?
0 / 5 - 0 ratings

Related issues

TysonMN picture TysonMN  路  4Comments

andrevdm picture andrevdm  路  4Comments

martasp picture martasp  路  3Comments

michael-wolfenden picture michael-wolfenden  路  5Comments

chyczewski-maciej picture chyczewski-maciej  路  4Comments