BagPartDisplayDriver is throwing IndexOutOfRangeException

[MikeAlhayek]

Describe the bug

I have a content-type called 'Guide` and its created like the following

_contentDefinitionManager.AlterTypeDefinition("Guide", type => type
    .Listable()
    .Creatable()
    .Draftable()
    .Securable()
    .Versionable()
    .WithPart(nameof(TitlePart), part => part
        .MergeSettings<TitlePartSettings>(settings =>
        {
            settings.RenderTitle = true;
            settings.Options = TitlePartOptions.EditableRequired;
        }))
    .WithPart(nameof(LenderPart))
    .WithPart(nameof(AgreementBoundaryPart))
    .WithPart(nameof(DescriptionPart))
    .WithPart("Evaluation", nameof(BagPart), part => part
        .WithDescription("")
        .WithDisplayName("Evaluations")
        .MergeSettings<BagPartSettings>(settings =>
        {
            settings.ContainedContentTypes = new[] { "Evaluation" };
        })
    )
    .WithPart("Plan", nameof(BagPart), part => part
        .WithDescription("")
        .WithDisplayName("Plans")
        .MergeSettings<BagPartSettings>(settings =>
        {
            settings.ContainedContentTypes = new[] { "Plan" };
        })
    )
);

Sometime when I add a new Plan to the guide and hit Save and Continue button my changes do not get saves and I get the following error in the logs

2021-12-11 17:28:39.3334|Default|00-b1cb527f118e7445aaeacc869c43523f-840de667a532084f-00||OrchardCore.ContentManagement.Display.ContentItemDisplayCoordinator|ERROR|IContentPartDisplayDriver thrown from OrchardCore.Flows.Drivers.BagPartDisplayDriver by IndexOutOfRangeException System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at OrchardCore.Flows.Drivers.BagPartDisplayDriver.<>c__DisplayClass6_0.<UpdateAsync>b__0(ContentItem x)
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Func`2 predicate, Boolean& found)
   at OrchardCore.Flows.Drivers.BagPartDisplayDriver.UpdateAsync(BagPart part, UpdatePartEditorContext context)
   at OrchardCore.ContentManagement.Display.ContentDisplay.ContentPartDisplayDriver`1.OrchardCore.ContentManagement.Display.ContentDisplay.IContentPartDisplayDriver.UpdateEditorAsync(ContentPart contentPart, ContentTypePartDefinition typePartDefinition, UpdateEditorContext context)
   at OrchardCore.ContentManagement.Display.ContentItemDisplayCoordinator.<>c.<<UpdateEditorAsync>b__9_0>d.MoveNext()
--- End of stack trace from previous location ---
   at OrchardCore.Modules.InvokeExtensions.InvokeAsync[TEvents,T1,T2,T3](IEnumerable`1 events, Func`5 dispatch, T1 arg1, T2 arg2, T3 arg3, ILogger logger)    at OrchardCore.Flows.Drivers.BagPartDisplayDriver.<>c__DisplayClass6_0.<UpdateAsync>b__0(ContentItem x)
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Func`2 predicate, Boolean& found)
   at OrchardCore.Flows.Drivers.BagPartDisplayDriver.UpdateAsync(BagPart part, UpdatePartEditorContext context)
   at OrchardCore.ContentManagement.Display.ContentDisplay.ContentPartDisplayDriver`1.OrchardCore.ContentManagement.Display.ContentDisplay.IContentPartDisplayDriver.UpdateEditorAsync(ContentPart contentPart, ContentTypePartDefinition typePartDefinition, UpdateEditorContext context)
   at OrchardCore.ContentManagement.Display.ContentItemDisplayCoordinator.<>c.<<UpdateEditorAsync>b__9_0>d.MoveNext()
--- End of stack trace from previous location ---
   at OrchardCore.Modules.InvokeExtensions.InvokeAsync[TEvents,T1,T2,T3](IEnumerable`1 events, Func`5 dispatch, T1 arg1, T2 arg2, T3 arg3, ILogger logger)

I do get Your Guide draft has been saved. on the screen even when it fails.

This issue happens in a production all contently and I can't seems to reproduce it locally. The only think I have to go about is the logs from the production. Locally, most of the time is works. However, sometimes it did not and in the logs file I got the same error as the above.

First of all. why OC process the save content-item even when a driver throws exception? I would think it should give the user some sort of error instead of giving a misleading success message!

How can I narrow down or fix the problem?

[Skrypt]

Provide a repro is first step. Else, we can't really guess what's happening.

[jtkech]

@CrestApps

Not sure at all but can you retry by giving different names to your bag parts, not the same name as the content type of the embedded items, e.g. not Evaluation but Evaluations in the plural form.

Also use WithSettings() if it is a first migration, not an UpdateFromX.

[MikeAlhayek]

@jtkech I did not make any change yet. I deleted the app from production and redeployed it and now it seems to be working again not sure what was the issue. I am not sure if there is a way to change the name of the part at this point since my databases already have data in them, is there an easy way?

Also, why would OC process the save content-item even when a driver throws exception? I would think it should give the user some sort of error instead of giving a misleading success message!

[jtkech]

Yes, no easy way

Yes, it cancels the session if the validation fails, the session is also canceled when a non managed exception is thrown in a given scope. But handlers / drivers are safely called through an InvokeAsync helper that uses try catch blocks, the errors are logged but yes the session is not canceled.

[MikeAlhayek]

Yes I am aware of the behavior. I am just wondering why it does that? If a driver fail to save due to an exception, stating that the content was saved, is misleading. An error message should be shown instead to eliminate the confusion. If an error is shown instead, the admin could evaluate the logs... but stating success is invalidate since nothing was saved.

[sebastienros]

So the Driver is not correctly indicating that the model is invalid?

[sebastienros]

Bug is that the index i is not valid for the collection model.ContentItems[i], fails the Linq query.

https://github.com/OrchardCMS/OrchardCore/blob/781ea118f2aed6b3c7276a4a33b5a32481c24648/src/OrchardCore.Modules/OrchardCore.Flows/Drivers/BagPartDisplayDriver.cs#L68-L71

[jtkech]

Yes, I saw that it failed here, when I updated it to read ContentItems[i] I thought about also looping on it so that it would never fail here, but said to me that it is better to still looping on Prefixes because the 2 collections should have the same length as the related form inputs are defined alongside in the same razor view. https://github.com/OrchardCMS/OrchardCore/blob/c405244f0c1a8593d6a7b95187e65478ee32a97b/src/OrchardCore.Modules/OrchardCore.Flows/Views/FlowPart.Fields.Edit.cshtml#L1-L3

Hmm maybe some null values resulting in 2 submitted collections of diferent lengths, I will think about a possible way to repro.

@CrestApps you said that it only happened in production after upgrading your version, and that was okay after redeploying, did you experienced again this exception?

Hmm, maybe a *.Views.dll was not updated correctly, e.g. the one defining the new ContentItems hidden inputs.

[MikeAlhayek]

@jtkech Yes it is no longer an issue for some reason. I am also suspected a bad *.Views.dll in production. After I deleted all dlls from production and republished, it is working correctly.