elmahio / elmah.io.umbraco

Integrates elmah.io with Umbraco CMS.
https://elmah.io
Apache License 2.0
5 stars 0 forks source link

The Elmah NotFoundLastChangeFinder overrides any other LastChanceContentFinders #2

Closed NikRimington closed 5 years ago

NikRimington commented 5 years ago

I'm using the following package in an Umbraco website: https://github.com/TimGeyssens/UmbracoPageNotFoundManager

The problem I'm having is sometimes when the site loads the application starting events for the Elmah.io.umbraco package kicks in after the PageNotFoundManager package overriding the LastChanceContentFinder in Umbraco.

Unfortunately Umbraco only allows 1 of these to execute. It would be great if there was a different way for Elmah.io to capture 404's without overriding this, or for it to be configurable to fall back to another ContentFinder after it has sent it's error notification.

ThomasArdal commented 5 years ago

@sitereactor Do you know if there is a better way to listen for 404s?

sitereactor commented 5 years ago

@zpqrtbnk would be a better fit for answering this. I'm not all that familiar with the possibilities in the ContentFinders. But I'm sure Stephane would be able to answer this.

NikRimington commented 5 years ago

Hi,

I'm assuming that there is no update on this unfortunately?

Nik

ThomasArdal commented 5 years ago

No update. Maybe Umbraco 8 will have some improvements for this @zpqrtbnk?

ThomasArdal commented 5 years ago

If Umbraco doesn't support this, an option would be to make elmah.io's LastChanceContentFinder configurable. This way you could decide which package to use. 404s wouldn't be logged to elmah.io though.

sitereactor commented 5 years ago

Now is not a good time to get a response from Stephane. He has more then enough on his plate with the upcoming v8 release, so I would suggest to try again in a month. Sorry!

NikRimington commented 5 years ago

That would be really useful to have it configurable as I'd rather have the custom 404 working than the 404 tracking in Elmah.io :-) It might be a case that I end up trying to create my own content finder that does both the 404 and then calls the Elmah.io code although I'm not sure. Just we keep getting complaints when the Elmah.IO stops the custom 404s from showing :-)

zpqrtbnk commented 5 years ago

Oops wasn't paying attention to this - Umbraco 8 composers can be ordered and register one after another... but... do I understand that elmah tracks 404 but handling them (as in, last chance finder)? meaning, you just cannot have another last chance finder?

then... what about elmah registering a normal finder at the last position, that would track the 404 but return it cannot find content, so that ppl last chance finder can still kick in?

ThomasArdal commented 5 years ago

You might be able to do something like this:

  1. Install Elmah.Io.WebApi.
  2. Add the ElmahExceptionLogger:
GlobalConfiguration.Configuration.Services.Add(typeof(IExceptionLogger), new ElmahExceptionLogger());
  1. Add elmah.io.log4net.

  2. Add the elmah.io.log4net config: https://github.com/elmahio/elmah.io.umbraco/blob/master/elmah.io.umbraco/NuGet/content/Config/log4net.config.install.xdt

  3. Implement your own IContentFinder:

public class CustomPageNotFoundContentFinder : IContentFinder
{
    public bool TryFindContent(PublishedContentRequest contentRequest)
    {
        if (contentRequest.Is404)
        {
            ErrorSignal.FromCurrentContext().Raise(new HttpException(404, "Page not found"));
        }

        return new PageNotFoundContentFinder().TryFindContent(contentRequest);
    }
}
ThomasArdal commented 5 years ago

@sitereactor No worries, Morten. Sounds like U8 may be slightly more important than supporting multiple content finders 😂

zpqrtbnk commented 5 years ago

mmm, but I did reply !

sitereactor commented 5 years ago

I guess this means v8 is done then 😄😄

ThomasArdal commented 5 years ago

@zpqrtbnk elmah.io registers itself as a ContentLastChanceFinder, yes: https://github.com/elmahio/elmah.io.umbraco/blob/master/elmah.io.umbraco/ElmahIoApplicationEventHandler.cs#L14. I wasn't aware that there were other ways to listen for 404s? At least not <U8.

ThomasArdal commented 5 years ago

elmah.io's IContentFinder returns false (https://github.com/elmahio/elmah.io.umbraco/blob/master/elmah.io.umbraco/ElmahIoApplicationEventHandler.cs#L29), but in the case where UmbracoPageNotFoundManager runs first, elmah.io's content finder is never invoked.

ThomasArdal commented 5 years ago

@NikRimington Does the solution proposed in this comment solve your problem? I think we will need to re-implement this for Umbraco 8 anyway since the API has been changed.

NikRimington commented 5 years ago

Hi @ThomasArdal , I've not had a chance to test it but it sounds like it should do as long as I can ensure that it is the last content finder to run before the 404 handler.

Just to confirm, I'd have to uninstall the "Elmah.IO.Umbraco" nuget right? Which is why I'd have to manually run the xdt for Log4Net you mentioned as that and the content finder are the only two actions that the pack performs correct?

ThomasArdal commented 5 years ago

@NikRimington You will need to uninstall Elmah.Io.Umbraco, yes. That package contains the last change content finder and automatically add it on startup. You can copy the log4net config you already have to re-use after uninstall. There is more help for log4net here: https://docs.elmah.io/logging-to-elmah-io-from-log4net/.

NikRimington commented 5 years ago

Okay, so I've been testing this over the last day or so and I've been able to use the ideas discussed here to get something that appears to work. What I'm actually doing is creating my own content finder that at start up is injecting itself at the very end of the ContentFinder lists and not as the last chance finder. This appears to work nicely :-)

ThomasArdal commented 5 years ago

Good to hear that, Nik. I'll go ahead and close this issue then. Hoping to do something better for Umbraco 8, once I find a good alternative to content finders there.