Forward Opts Type Constraints backwards?

[netogallo]

I am using Akka.Net 1.4.0-beta3 and I am also using the documentation here: https://getakka.net/api/Akka.Streams.Dsl.ForwardOps.html

From what I can see, the ForwardOpts "To" methods always constrain TIn : TOut. This allows this to compile:

GraphDsl.Create<ClosedShape>(
    builder =>
        {
                 builder.From(Source.Single<object>(""))
                      .To(builder.Add(Sink.Create<string>(null)));

                 return ClosedShape.Instance;
         });

This to me is wrong since a string sink can get things that are not strings as the object Source can basically emit whatever it wants. Furthermore, the compiler would reject this:

GraphDsl.Create<ClosedShape>(
    builder =>
        {
                 builder.From(Source.Single<string>(""))
                      .To(builder.Add(Sink.Create<object>(null)));

                 return ClosedShape.Instance;
         });

Which in my opinion is perfectly fine since a object sink should be able to be provided with strings.

Sorry if this issue is a dupe and I hope it helps. Thanks for the good work and great library.

[Aaronontheweb]

.... Yeah this might be wrong.

cc @IgorFedchenko ?

[IgorFedchenko]

Seems like it is more complex then just swapping types - see discussion here: https://github.com/akkadotnet/akka.net/pull/4515#discussion_r454595146

We have Interleave flow that actually is build from such base-to-child types flow, and it works well...

@netogallo Are you able to actually build graph, that will compile, but will throw runtime exception when passing messages to it due to this issue? I.e. if you are building graph from first example, and passing some non-string object, do you get issues with it actually?

Asking about this because to me it seems like Interleave and some other flows are doing this, and working well... Which is little but weird to me, but probably it is not a bug, but some not obvious way of how things work internally.

[netogallo]

Thanks @IgorFedchenko for looking into this. I wrote this small test using the TestKit and Xunit with the latest stable release of Akka (1.4.8):

        [Fact]
        public void TestWierdGraph() {

            var g = GraphDsl.Create(
                builder => {
                    
                    // Scenario 1
                    var sr = builder.From(Source.Repeat(new object()));
                    
                    // Scenario 2
                    // var sr = builder.From(Source.Repeat("hello")); // Scenario 2

                    var sk = builder.Add(Sink.ForEach<string>(s => Console.WriteLine(string.Concat(new []{ "yes", s, s }))));
                    sr.To(sk);

                    return ClosedShape.Instance;
                });

            RunnableGraph.FromGraph(g).Run(Sys.Materializer());

            Task.Delay(5000).Wait();
        }

It has two scenarios and this is what we observed when each of them is used:

• Scenario 1: The code compiles and executes without any error but nothing happens. Not sure if this is good or bad as the code is incorrect but the user would not get any compile time or runtime error, even if nothing happens.
• Scenario 2: Does what one would expect

Hope this helps. Also, just letting you know that I am not impacted at all by this bug. It was by chance that I found it and simply reported it with the intention of improving the library :)

[netogallo]

One option I can think of is to leave the current API untoched but make everything internal and change the constraints in the user-facing API. I however, am not sure if the fact that this is used internally in other places might result in other bugs.

[IgorFedchenko]

Well, codebase, especially such "core" part like Akka.Streams, is well-covered with tests here. I am more conserned about breaking changes here in public API... But should not be a problem actually - probably if someone uses such "wrong" source-sink types, he will be happy to start getting compiler issues here.

@netogallo Thanks for reporting, and writing test for this! We should probably get back to this once will have more time, and improve types handling.

Someone can also pick up my PR - I have finished type constraints swap, but need to change few internal graphs implementation accordingly. Could not do that quickly enough at the moment.

[netogallo]

Excellent! I think it will help users.

I am not sure if this anecdote is related to the issue, but I was experimenting with using the GraphDSL to work with graphs that materialize to things other than NotUsed. I unfortunately don't have the original code but I do remember that I ended up in similar situations where inlets & outlets of unlatching types somehow became connected resulting in graphs that didn't do anything.

If you have any other quesitons, let me know!

[netogallo]

Also, one thing worth noticing is that these "wierd" graphs can only be constructed with the GraphDSL, in other words, something like:

Soruce.Repeat<object>(new object()).To(Sink.ForEach<string>(s => Console.WriteLine(s))).Run(...)

Is rejected by the compiler which at a minimum makes the public facing API inconsistent.