-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NullReferenceException accessing httpContext within WorkContextAccesor when it is called from a Migrations #5490
Comments
You shouldn't use the content manager in migrations, because migrations are executed very early when the application is started and is not yet completely initialized. If you really need to use services which rely on the work context in a migration, then you can try to create a work context scope manually. |
In previous versions it was possible to use ContentManager at Migrations. Is not there a possible workaround to Activate AutomaticDataMigration after work context has been initialized? I followed your advice attempting to get a ContextScope without success:
I get the following Autofac Exception:
|
ContentManager should be usable from migrations just as before (and shell event should be wrapped into WCs #4852). I think @sfmskywalker can help with this. |
Yes, regardless of whether or not it is good practice to use |
Hello Sipke Schoorstra. |
I'm afraid not, I have yet to look into it. Try and inject it as a |
Inject it as |
Sounds good. Have fun. ;-) |
@jersiovic, here's how I create work context scopes in my Orchard PowerShell project: I'm not sure if this is the best way to do it, but it works. |
Hello. And added the old constructor to HttpContextPlaceholder class
Is not a solution but i can now make my import.... |
Thank both for you advice
@MpDzik I will give a try to your solution
|
After posting previous comment I looked twice at the point where exception is thrown (at WorkContextModule.Load() method) I have tried to registerWorkContextProperty for "shell" lifetimescope, that is the one is been used to resolve it :
Now it works!! :) |
What do you think of this variation of your solution @jafcampos ? Instead of replacing HttpContextBaseFactory(IComponentContext context) method:
In that way there is not need of adding the old constructor to HttpContextPlaceholder class I've tested it and it seems to work properly. |
It looks this doesn't solves all problems with context.
The exception:
|
I jersiovic. Or there are other exceptions? Since i made my code changed (workaround) i haven't got any exception. |
Yes I did that. It solved problem in migrations. But I found problems with context in other scenarios like I describe in previous post. I'm not sure if it is a colateral problem of this workaround. I will make some tests with a site from scratch to isolate the problem before con back with more info. |
Finally problem is in this version we cannot access to So yes, the proposed solution seems to fix the problem. |
Looking at the commit which might have broken this, the code looks crazy, or at least I will need some time to understand the impact. Linked for ref: ce630f9 |
FYI: That ref is a tad outdated. |
Try this, today I got into very similar problem.
|
Thank you @kumards I understand my solution of adding ...InstancePerMatchingLifetimeScope("shell") is not good because breaks Orchard. So finally only solution I have found is to resolve ContentManager just when I'm going to use it as you @kumards proposed Problem of this solution is I have more code in migrations of other modules which uses ContentManager. This code is executed when a new tenant is created, so I will need to fix also that code. The other problem I see is for new comers to Orchard, this solution is something not obvious. So is likely they waste time trying to understand why ContentManager fails in migrations. |
Take into account problems are not only related with ContentManager, those also happen with WidgetsService.GetLayers(). Furthermore, with this solution what happens when migrations fails? Operations done in a different scope won't be rolled back |
I also just ran into this bug :( |
I am just curios, what happens when the ContentManager is injected as Work object in the constructor? |
@kumards if do you mean to this, it is something I commented on the third comment on this thread
|
I faced this problem on July 9 when I was completing an upgrade of a multitenant site from version 1.7 to 1.9.1 which will be updated automatically using migrations + recipes + commands. The point is after two months I'm still stuck at the same point, and I supporting pressure for upgrading to 1.9.1 On Tuesday meeting Committee was said @sfmskywalker was going to undo changes on a previous commit to solve this issue. Do you have any approximate idea of when it will be solved? |
I've been battling this problem myself too since upgrading to VS 2015. In fact it happens for me not only for 1.9 sites but for 1.8 sites that I have to maintain too, when building under VS2015. As @Piedone mentioned above, #4852 is the real problem here because there is no work context when Migrations are being run using Automatic Data Migrations. I have implemented a fix to #4852 locally and this has solved the problem for me, in both 1.8 and 1.9. I'll submit a pull request for #4852 in a few hours. It does worry me that this problem only occurs under VS2015, and not under any previous Visual Studio version. Does anyone know the reason for this? |
Cool, I will be looking forward to hearing from you. |
The reason I started refactoring @sebastienros proposed that we roll back all of my changes to see if the issue still exists with VS2015 RTM. I tried yesterday, and sadly, the issue is still there. I reported this to Sebastien and he'll digg into why the exact same code works when built with VS2013 but not with VS2015. This is the commit hash to reproduce the issue with To recap, in VS2013 everything up to and including d3ccd74 runs fine, but fails in VS2015. I can think of the following course of actions:
|
Thank you @sfmskywalker at least now things are more clear |
@jersiovic No, the below implementation is wrong.
Because, the If you refer to my comment here #5490 (comment), you will see the implementation difference. What I meant about injecting ContentManager as Work object was something like below
|
I've submitted a pull request. |
I have submitted the VS discrepancy to someone at Microsoft. I will keep you posted. |
Running 1.9.x branch (08/07/2015 commit) when a migrations like this one is executed:
I get NullReference exception at this method at WorkContextAccessor because httpContext is null:
Looking at the stacktrace problem comes at MvcModule.HttpContextBaseFactory() method when httpContextBase is resolved because it returns null:
It seems that httpContextBase is not properly instantiated when the call is not initiated by a webrequest, as happens when migrations code is run.
Any idea on how to fix this?
The text was updated successfully, but these errors were encountered: