-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Publishing: Resolve exceptions on publish branch #20464
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
Conversation
…th expected exceptions.
…d such that UmbracoContext is available.
… can get an IUmbracoContext in the URL providers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't you also add the following to the start of ContentPublishingService.PerformPublishBranchAsync()?
using UmbracoContextReference umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext();This way the code that is called after should be able to retrieve a context from the accessor, and you shouldn't need to update all of accessor calls in the url providers.
|
Thanks @lauraneto - yes, it seems that works fine, and is quite a bit simpler. I've applied your suggestion and it looks to have resolved the problem to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally and it now works as expected!
* Reduce log level of image cropper converter to avoid flooding logs with expected exceptions. * Don't run publish branch long running operation on a background thread such that UmbracoContext is available. * Revert to background thread and use EnsureUmbracoContext to ensure we can get an IUmbracoContext in the URL providers. * Updated tests. * Applied suggestion from code review. * Clarified comment.
Prerequisites
Description
This PR resolves a couple of exceptions found in testing the publish branch feature.
Firstly I was seeing errors in
UrlProviderwhich requires anUmbracoContext. When publishing a branch with unpublished descendants and a webhook enabled on publish, this would get called, as the webhook payload was being created.I've amended this now so we ensure an Umbraco context exists if one can't be accessed.
Secondly we were logging errors on seemingly expected scenarios with the image cropped property value converter. So here I've reduced the log level to debug and tightened up the try/catch so we only handle the specific error.
Testing