Skip to content

Conversation

@AndyButland
Copy link
Contributor

@AndyButland AndyButland commented Oct 10, 2025

Prerequisites

  • I have added steps to test this contribution in the description below

Description

This PR resolves a couple of exceptions found in testing the publish branch feature.

Firstly I was seeing errors in UrlProvider which requires an UmbracoContext. 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

  • Set up a webook to fire on content publication.
  • Unpublish a child item.
  • Publish the parent item with descendants.
  • Verify that the operation completes without exceptions being thrown.

Copilot AI review requested due to automatic review settings October 10, 2025 14:07

This comment was marked as outdated.

… can get an IUmbracoContext in the URL providers.
@AndyButland AndyButland requested a review from Copilot October 10, 2025 17:54
Copy link
Contributor

Copilot AI left a 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.

Copy link
Contributor

@lauraneto lauraneto left a 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.

@AndyButland
Copy link
Contributor Author

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.

Copy link
Contributor

@lauraneto lauraneto left a 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!

@lauraneto lauraneto merged commit daace4b into main Oct 21, 2025
24 of 25 checks passed
@lauraneto lauraneto deleted the v16/bugfix/exceptions-on-publish-branch branch October 21, 2025 10:05
lauraneto pushed a commit that referenced this pull request Oct 21, 2025
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants