Implement async option in Optional.Async

[nlkl]

An async optional will make it more convenient to combine Optional with async/await.

[kodfodrasz]

Hi!

Could you please publicate the AsyncOption - at least as a prerelease - NuGet package?
It would come handy for me right now.

Thx.

[nlkl]

Hi,

I am afraid the AsyncOption isn't in a state where it is ready to be published - not even as a prerelease package.

Unfortunately I haven't had a lot of time to work on it lately, but I hope I will get some time to finish the implementation in the not-too-far future.

Meanwhile, your best bet is probably to clone the project and build the DLLs yourself.

- Nils

[TimGraf]

Since moving from Scala to c# and .net core, I was excited to find this project. I'd definitely be interested in the AsyncOption feature as well. Great project by the way.

Cheers,
Tim

[JudahGabriel]

Is there a reason AsyncOption uses .ConfigureAwait(false), instead of .ConfigureAwait(true)? It causes breakage in web apps that expecting the same thread (e.g. to access the current request) after awaiting an Async Option.

For example:

public async Task<ActionResult> Index()
{
   var result = await someAsyncOption;

   // OH NO! This breaks; there is no request because the await didn't continue on the captured context.
   var requestFoo = Request.RawUrl; // Exception: Request is unexpectedly null
}

This would also break in desktop app contexts including WinForms and WPF, as the continuation will be resumed on a different thread.

Would you accept a pull request that fixes this?

[candoumbe]

@JudahGabriel
Maybe you should
var result = await someAsyncOption.ConfigureAwait(true) in your case ?

[JudahGabriel]

I would have to place that in every controller method where Optional is used. (Or in a desktop app, in every UI event handler.)

That's both error prone and ugly.

How about we just change it to .ConfigureAwait(true) inside AsyncOption code? Is there a good reason it's set to false currently?

[nlkl]

Hi @JudahGabriel and @candoumbe

This is a very real concern - that is whether to continue on the captured sync context when executing async lambda expressions (e.g. when mapping the option) - and one that I never found the perfect solution for. I haven't had much time to consider AsyncOption lately, so the question hasn't really received too much attention since then, I'm afraid.

In general, the problem is that there are situations where both of the two options would be preferred. E.g. one might need to access the current HttpContext in a call, in which case the context is needed - but on the other hand, the overall consensus is to prefer ConfigureAwait(false) whenever the context isn't explicitly needed (to potentially avoid deadlocks and improve performance in some cases).

For regular Tasks this is solved by letting the consumer decide whether to capture the context or not when the task is awaited (by calling ConfigureAwait). If not explicitly chosen, it will default to true (continuing on the captured sync context), although it would in my opinion be a more sane default if it had been the other way around.

Anyway, my current opinion is that an async option will be of limited use, if the consumer is unable to choose whether to capture the context or not. As such, I am currently considering adding an overload to all methods taking an async action/func as a parameter, letting the consumer decide.

This approach is similar to that of Polly (https://github.com/App-vNext/Polly/wiki/Asynchronous-action-execution#async-synchronization-context) - and like Polly, Optional will likely stick to keeping ConfigureAwait(false) the default, requiring the consumer to explicitly opt in whenever the context is needed.

I am still considering a few other options, so nothing is set in stone. I am currently focusing on releasing Optional 4.0.0, which should be just around the corner, after which I will revisit Optional.Async - and hopefully have it ready in the not-so-distant future either (although an async Option requires quite a bit of boilerplate, and similar testing, so it will be some time still).

Hope it clarifies things a bit.

Kind regards,
Nils

[JudahGabriel]

I'm cool with it being configurable, provided it can be configured once at startup. Virtually all AspNet, WPF, and WinForms apps would want AsyncOptional to continue on the captured context by default, for example.

So if we can opt-in to that behavior once, that's acceptable.

Would you accept a PR that makes this configurable, leaving the current default in place?

[nlkl]

Since Optional doesn't carry any global state, it is unlikely that a one-time configuration will be available I'm afraid.

In my experience, 90% of my async calls tend to use ConfigureAwait(false), even in ASP.NET and desktop applications. It certainly depends on how you structure your code etc. (e.g. how much the code depends on global state like the HttpContext), so I will not try and convince anyone that this is the case for all solutions.

Nevertheless, I do believe that ConfigureAwait(false) is the better default in general. I will, however, try and keep the ergonomics as good as possible in both cases, even if capturing the context will be explicit and add a bit of noise.

/Nils

[nlkl]

Regarding the PR, I think the whole Optional.Async codebase needs a bit of cleanup before I start taking pull requests - to not waste any of your time, as things might get rewritten quite extensively once I get around to it.

/Nils

[JudahGabriel]

OK, I'll leave it for now.

I can say I've been happily using Optional.Async for a few months now in 3 different AspNetCore projects. For all 3 projects, I just added a global configuration option allowing me to specify .ConfigureAwait(true) everywhere in Optional.Async. In my use case, it's needed not only for HttpContext access but also for database contexts which track objects based on the current thread.

Should you move forward with Optional.Async in the future, I'd vote for a global configuration option, rather than having to litter hundreds of .ConfigureAwaits throughout my controllers.

[kodfodrasz]

The following MSDN article suggests using ConfigureAwait(false) in libraries whenever possible.
https://msdn.microsoft.com/en-us/magazine/jj991977.aspx

I understand that sometimes it is not the desired default, but what I seen in my works is that this is more often a good choice.

[RaringCoder]

@nlkl Any movement on this? I keep running into the "I need async" issue too. More than happy to muck in and do some of the dev work if you need a hand. For now I have to drop out of using Optional if I need to perform asynchronous tasks.

[kodfodrasz]

I gave up using Optional, partly for this (lack of proper async support), and also for the lack of covariance, which together held meg back more than they helped.

[nlkl]

@RaringCoder - Unfortunately not yet. I'm afraid I have been pushing it aside for some time, as I thought the interest was rather limited. However, from this thread (and other sources) it does seem the interest is there and growing, and as such, finishing up Optional.Async will be my highest priority as soon as v4.0 is out (which shouldn't be long now - there was a minor delay due to some late summer vacation plans).

@kodfodrasz I am sorry to hear that. Hopefully, once Optional.Async is out, you will consider giving it another try. Regarding the lack of covariance, this is a rather unfortunate shortcoming of C#, and nothing I can easily change without sacrificing Option being a struct - even if this change is rather unlikely to be introduced, I am quite keen to hear your opinion on the matter: would covariance be worth it, even if it meant that an Option would be nullable (passed around as an IOption interface more specifically)?

/Nils

[JudahGabriel]

Nils: I vote keep Option as a struct. Covariance not worth it.

As for async optional, I'm still happily using Optional.Async. I just downloaded your source files, added them to my project. To make it place nice with ASP.NET Core, I modifed them to .ConfigureAwait(true).

[kodfodrasz]

@nlkl we used Option in application code dealing with other microservices, and database, handling the possible failures, and also short-circuiting logic sometimes. After all the lack of covariance was not that troubling, apart from a few painful cases, as Task<T> is also invariant, and that was the main cause to give up Option for now.

There were some cases where we had to manually upcast some objects to base types, especially in the Option<T, TException> case, when there was a base exception type, and several more refined derived types allowing finer grained error handling/reporting, and exceptions had to be upcast (or Option type explicitly specified) at several places, which hindered readability pretty much.

The problem with this was that we hoped that the use of Optional there would rather improve readability of the code, as we could explicitly specify the possible errors this way. (think about a piece of code that must access several resources across integration boundaries, which can fail in a multitude of ways, and extensive error handling is necessary)

Maybe we wanted to use this particular hammer for a problem not being a nail, or the intrinsic complexity of that problem was too hard so it was naive to hope this to remove too much of that.

Overall I liked Option, but also this was not a personal project, and it turned out that there it is not worth for us in the current form.

Regarding structs and non-nullability: It is promised that C# 8.0 will have non-nullable references! Maybe it could be considered then. I don't wish to have a value given up for my preference, given that currently I'm not using the library. I'm just stalking here, as I liked it, and hope that I will use it again in the future.

[rydergillen-compacSort]

First off I want to say I'm a big fan of the Optional library. I have been using it for about 2 years. However like many I have found it cumbersome to consume Async methods while maintaining a fluid style of programming. About 6 months ago I developed the TaskOption<T> type as a compliment to the existing optional library.

• Implicitly convertible to/from Task<T> for simple method return types of Task<T>
• No use of async/await when returning another Task<T>
• await returns Option<T, Exception>
• No internal use of await, caller can invoke ConfigureAwait() the same as Task<T>

Works seamlessly on any existing Task<T> type.

var taskOption = 
GetJsonAsync()
.ToTaskOption()
.MapException(ex => new Exception($"API Call Failed", ex))
.Map(json => JsonSerializer.Deserialize<Employee>(json))
.Filter(employee => employee.IsSupervisor, some => new Exception($"{employee.Name} is NOT a Supervisor"))

await to return to normal Option<T, Exception> then MapException()
Task<T> task = taskOption; //can be directly assigned to Task<>
Option<T, Exception> option = await taskOption;
Option<T, string> stringErrorOption = option.MapException(ex => ex.Message);
Option<T> emptyErrorOption = option.WithoutException();

public class TaskOption<T>
{
	private readonly Func<Task<T>> _task;

	public TaskOption(Task<T> task)
		: this(() => task)
	{ }

	public TaskOption(Func<Task<T>> task)
	{
		this._task = task ?? throw new ArgumentNullException(nameof(task));
	}

	public TaskOption<T> Filter(Predicate<T> filterPredicate, Func<T, Exception> exceptionalFunc)
	{
		return this.Match(
			some: s => filterPredicate(s) ? s : throw exceptionalFunc(s),
			none: n => throw n);
	}

	public TaskOption<TResult> Map<TResult>(Func<T, TResult> mapping) =>
		this._task().ContinueWith(t => mapping(t.Result));

	public TaskOption<TResult> Map<TResult>(Func<T, Task<TResult>> mapping) =>
		this._task().ContinueWith(t => mapping(t.Result)).Unwrap();

	public TaskOption<TResult> Match<TResult>(Func<T, TResult> some, Func<Exception, TResult> none) => this._task()
		.ContinueWith(t =>
		{
			if (t.IsCanceled)
			{
				return none(new TaskCanceledException(t));
			}

			if (t.IsFaulted)
			{
				return none(t.Exception);
			}

			return some(t.Result);
		});

	#region Await

	public TaskAwaiter<Option<T, Exception>> GetAwaiter()
	{
		var continued = this._task().ContinueWith(t =>
		{
			if (t.IsCanceled)
			{
				return Option.None<T, Exception>(new TaskCanceledException(t));
			}

			if (t.IsFaulted)
			{
				return Option.None<T, Exception>(t.Exception);
			}

			return Option.Some<T, Exception>(t.Result);
		});

		return continued.GetAwaiter();
	}

	public ConfiguredTaskAwaitable<Option<T, Exception>> ConfigureAwait(bool continueOnCapturedContext)
	{
		var continued = this._task().ContinueWith(t => {
			if (t.IsCanceled)
			{
				return Option.None<T, Exception>(new TaskCanceledException(t));
			}

			if (t.IsFaulted)
			{
				return Option.None<T, Exception>(t.Exception);
			}

			return Option.Some<T, Exception>(t.Result);
		});

		return continued.ConfigureAwait(continueOnCapturedContext);
	}

	#endregion

	#region Operators

	public static implicit operator Task<T>(TaskOption<T> option) => option._task();

	public static implicit operator TaskOption<T>(Task<T> task) => new TaskOption<T>(task);

	#endregion
}