UiPath / CoreWF

WF runtime ported to work on .NET 6
MIT License
1.13k stars 217 forks source link

WorkflowApplication.ResumeBookmark starts workflow from beginning #55

Closed zehavibarak closed 5 years ago

zehavibarak commented 5 years ago

Resuming a bookmark should not call Activity.Execute at all. Instead, it should only call the callback.

zehavibarak commented 5 years ago

I'm using a guid bookmark for context.CreateBookmark(...). The resume repeats this call, overriding the old bookmark with the new one.

lbargaoanu commented 5 years ago

A PR is welcome.

zehavibarak commented 5 years ago

The ResumeBookmark below calls the activity Execute, this is wrong

            WorkflowApplication wfApp = CreateWorkflowApplication();
            var workflowInstanceId = wfApp.Id;
            wfApp.Run();
            string bookmarkName = workflowInstanceId.ToString();
          _unloadedEvent.WaitOne();
/* create */                    
             wfApp = CreateWorkflowApplication();
             wfApp.Load(workflowInstanceId);
             var result = wfApp.ResumeBookmark(bookmarkName, null); // <- this hits the BookmarkActivity while **IT SHOULD'NT**

        public static WorkflowApplication CreateWorkflowApplication()
        {
            Activity wf = CreateWorkflow();
            WorkflowApplication result = new WorkflowApplication(wf)
            {
                Unloaded = e =>
                {
                    _unloadedEvent.Set();
                },
                PersistableIdle = e => PersistableIdleAction.None
            };

            return result;
        }

        public static Activity CreateWorkflow()
        {
            Sequence workflow = new Sequence();
            workflow.Activities.Add(
                new WriteLine
                {
                    Text = "Before Bookmark"
                });
            workflow.Activities.Add(new BookmarkActivity());
            workflow.Activities.Add(
                new WriteLine
                {
                    Text = "After Bookmark"
                });

            return workflow;
        }

public class BookmarkActivity : NativeActivity
    {
        protected override void Execute(NativeActivityContext context)
        {
            Guid wfInstanceId = context.WorkflowInstanceId;
            string bookmarkName = wfInstanceId.ToString();

            Console.WriteLine("BookmarkActivity.Execute - creating bookmark with name {0}", bookmarkName);
            context.CreateBookmark(bookmarkName, new BookmarkCallback(BookmarkCallback), BookmarkOptions.MultipleResume);
        }

        protected override bool CanInduceIdle => true;

        private void BookmarkCallback(NativeActivityContext context, Bookmark bookmark, object bookmarkData)
        {
                Console.WriteLine($"Bookmark {bookmark.Name} resumed with data that is not a string");
        }
    }
ewinnington commented 5 years ago

I've just started investigating the behaviour. For the moment, I have started a repro in my Issue55 github repo. I see the Framework version having the following output:

Before Bookmark
BookmarkActivity.Execute - creating bookmark with name 55a15653-e394-4ce1-83c1-6194a15558e1
Bookmark 55a15653-e394-4ce1-83c1-6194a15558e1 resumed with data that is not a string

but it doesn't execute the After Bookmark, I don't know if that is related to my instance store, which I copied from the WF examples.

As for the CoreWF repro on .net core, I'm stuck because the implementation of the InstanceStore uses a NetDataContractSerizalizer which is not available on netcore. I'll see if I can workaround that and get to the point of testing it.

I'm travelling for the next few days, so I'll see if I have any time to go further.

@zehavibarak Do you have a complete reproduction on NetFramework and NetCore that I can clone to investigate?

zehavibarak commented 5 years ago

The store may have failed to persists, thus code never get’s to event WaitOne.

Either way, the code in Execute run twice! 1st when the bookmark is created, which is as expected, 2nd when ResumeBookmark is called – when it shouldn’t. The code sample will work, but only because the bookmark name is set again in 2nd call, else overriding the original bookmark. If you try to set a bookmark created, say, from Guid.NewGuid().ToString(), store it, and try to resume from it – you will fail. Easy way to see it – debug code and you’ll hit Execute after ResumeCallback, which should not happen to my understanding – just the Callback.


From: Eric Winnington notifications@github.com Sent: Thursday, May 30, 2019 1:42:39 AM To: UiPath/corewf Cc: zehavibarak; Mention Subject: Re: [UiPath/corewf] ResumeBookmark calls Activity.Execute prior to calling callback (#55)

I've just started investigating the behaviour. For the moment, I have started a repro in my Issue55 github repohttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fewinnington%2Fbookmark-issue-55&data=02%7C01%7C%7C80b3dc7e16e1442e1ed208d6e486f4e5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636947665608973618&sdata=dm6k3r9SHoiRztwNHomNv%2Fa9qSW48D2mY8XjoG2ZUmE%3D&reserved=0. I see the Framework version having the following output:

Before Bookmark BookmarkActivity.Execute - creating bookmark with name 55a15653-e394-4ce1-83c1-6194a15558e1 Bookmark 55a15653-e394-4ce1-83c1-6194a15558e1 resumed with data that is not a string

but it doesn't execute the After Bookmark, I don't know if that is related to my instance store, which I copied from the WF examples.

As for the CoreWF repro on .net core, I'm stuck because the implementation of the InstanceStore uses a NetDataContractSerizalizer which is not available on netcore. I'll see if I can workaround that and get to the point of testing it.

I'm travelling for the next few days, so I'll see if I have any time to go further.

@zehavibarakhttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fzehavibarak&data=02%7C01%7C%7C80b3dc7e16e1442e1ed208d6e486f4e5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636947665608983626&sdata=dt%2BQE5zHfU8IGHZogHfh1f%2FkJNyl%2FpUYGAH2p3C%2BfhU%3D&reserved=0 Do you have a complete reproduction on NetFramework and NetCore that I can clone to investigate?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FUiPath%2Fcorewf%2Fissues%2F55%3Femail_source%3Dnotifications%26email_token%3DAAS5AEBWIY7YFY2RKZGBVQ3PX4BF7A5CNFSM4HQLSAU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWQ235I%23issuecomment-497135093&data=02%7C01%7C%7C80b3dc7e16e1442e1ed208d6e486f4e5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636947665608983626&sdata=JOVGh9TRITPbbIrb4zofwGWJCE9FAPzH9vzteyLaq20%3D&reserved=0, or mute the threadhttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAS5AEAXVC4LATGCBHKGLX3PX4BF7ANCNFSM4HQLSAUQ&data=02%7C01%7C%7C80b3dc7e16e1442e1ed208d6e486f4e5%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636947665608993635&sdata=7g3jGO8%2B79ogbkyHARuwnwTPDOX%2BigEZfEtqa7Urt9M%3D&reserved=0.

zehavibarak commented 5 years ago

This will fail.

    public static class Program
    {
        private static readonly AutoResetEvent _unloadedEvent = new AutoResetEvent(false);

        public static void Main(string[] args)
        {

            var wfApp = CreateWorkflowApplication();

            wfApp.Run();
            var workflowInstanceId = wfApp.Id;
            _unloadedEvent.WaitOne();
            /* new scope */
            wfApp = CreateWorkflowApplication();
            wfApp.Load(workflowInstanceId);

            Console.WriteLine("copy guid above here");
            string bookmarkName = Console.ReadLine();

            var result = wfApp.ResumeBookmark(bookmarkName, null);

            if (result != BookmarkResumptionResult.Success)
            {
                Console.WriteLine("Failed!");
            }

            Console.ReadLine();
        }

        public static WorkflowApplication CreateWorkflowApplication()
        {
            Activity wf = CreateWorkflow();
            var result = new WorkflowApplication(wf)
            {
                InstanceStore = new FileInstanceStore("."),
                Unloaded = e => _unloadedEvent.Set(),
                PersistableIdle = e => PersistableIdleAction.Unload
            };

            return result;
        }

        public static Activity CreateWorkflow()
        {
            Sequence workflow = new Sequence();
            workflow.Activities.Add(
                new WriteLine
                {
                    Text = "Before Bookmark"
                });
            workflow.Activities.Add(new BookmarkActivity());
            workflow.Activities.Add(
                new WriteLine
                {
                    Text = "After Bookmark"
                });

            return workflow;
        }
    public class BookmarkActivity : NativeActivity
    {
        protected override void Execute(NativeActivityContext context)
        {
            string bookmarkName = Guid.NewGuid().ToString();

            context.CreateBookmark(bookmarkName, new BookmarkCallback(BookmarkCallback), BookmarkOptions.MultipleResume);
            Console.WriteLine("BookmarkActivity.Execute - successfull created bookmark with name {0}", bookmarkName);
        }

        protected override bool CanInduceIdle => true;

        private void BookmarkCallback(NativeActivityContext context, Bookmark bookmark, object bookmarkData) => Console.WriteLine("Bookmark {0} resumed.", bookmark.Name);
    }
    }
}
zehavibarak commented 5 years ago

so ResumeBookmark doesn't work. It only appears to work since it calls Execute, creating a new bookmark, and only then calls its callback. If you pass a unique bookmark name, as does the code above, the bookmark resume returns NotFound result.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.