Feature request: Global way to catch exceptions

[nulltoken]

Is your feature request related to a problem? Please describe. Today, if I read the doco right, in order to process potential exceptions, notifications are to be used.

In this context, given 3 jobs, I have to register 3 different notification handlers for the sole purpose of logging exceptions.

However, more often than not, when an exception reaches out of a job, all we can really do is log it in an telemetry container to examine it further.

Describe the solution you'd like Ideally, being able to register one type through an interface that would be globally leveraged by all jobs for notification purposes, without having to link each job individually to it.

[linkdotnet]

In this context, given 3 jobs, I have to register 3 different notification handlers for the sole purpose of logging exceptions.

Yes - fair point and I like the feature request!

API-wise, how would you like something like:

builder.Services.AddNCronJob(o => {
  o.AddJob<MyJob>(...);
  o.AddGlobalExceptionHandler<MyHandler>();
});

It would follow ASP.NET in the sense that you can define multiple and can tell the system: "Hey, I handled everything, no need to rethrow or bubble up".

[nulltoken]

I'd :heart: that

[linkdotnet]

@nulltoken I will make a prerelease, if you want to playaroud. The basic semantics didn't change at all:

builder.Services.AddNCronJob(o => {
  o.AddJob<MyJob>(...);
  o.AddExceptionHandler<MyHandler>();
});

If the handler returns false the next in line will handle the exception. Returning true breaks the circuit.

The order of execution:

Job -> ExceptionHandler -> JobNotificationHandler -> Dependent Job

Independent if a job is handled or not, the JobNotificationHandler will see the exception passed in. Currently the flag is only to tell the system whether or not the next handler should be invoked.

Let me know your thoughts.

[linkdotnet]

Latest preview: https://www.nuget.org/packages/NCronJob/3.1.0-preview

[nulltoken]

@linkdotnet Great! Thanks a lot for this.

Just out of curiosity, do you have a branch pointer I could peek at? That would allow me to learn a bit more about the lib.

[linkdotnet]

@nulltoken

This time, because it was really straightforward I committed a sin and directly pushed to main. :D

Here the commit in question: https://github.com/NCronJob-Dev/NCronJob/commit/adaabb2e27ddafb7cf26f206b7783ea63e4a34de

[linkdotnet]

With DependentJobs and IExceptionHandler I may consider removing IJobNotifactionHandler in the future.

at least that was the original motivation for those - and with that feature, I don't see much benefit anymore.

[nulltoken]

Here the commit in question: adaabb2

Looks very neat!

Just a nitpick: In https://github.com/NCronJob-Dev/NCronJob/blob/adaabb2e27ddafb7cf26f206b7783ea63e4a34de/tests/NCronJob.Tests/GlobalExceptionHandlerTests.cs, shouldn't the test exception handlers write different values to the channel writer?

This may protect against potential future regressions would the order of execution change.

[linkdotnet]

Oh boy - yes. That happens if you just copy&paste stuff and test it once manually! Also the "true" case should be checked.

[linkdotnet]

Also - I will check for exceptions from the exceptionhandler. They shouldn't interfere with the system.

[nulltoken]

Also - I will check for exceptions from the exceptionhandler. They shouldn't interfere with the system.

Good catch! I didn't this of this. And it will eventually happen thanks to Murphy's law

[linkdotnet]

Indeed! I will push a new preview version 3.1.1-preview for this

[nulltoken]

I will push a new preview version 3.1.1-preview for this

I gave it a try. And that makes my life a lot easier. Thanks for that.

One thing that is missing (at least in my context), would be to get a bit more data about the Job that triggered the exception.

I can extract the instance Id from the context, but in order to make the log easier to parse, I'd like to decorate the telemetry with the Job type.

Is there a built-in way in NCronJob to retrieve this? Or would it be possible to potentially extend the context with an additional property?

[linkdotnet]

I am somewhat amazed that I didn't expose the jobName and jobType. See: https://docs.ncronjob.dev/advanced/dynamic-job-control/#job-names

Maybe that would help if it is part of the IJobExecutionContext?

[nulltoken]

Maybe it wasn’t needed before because surfaced in a job context?

However, I’d really appreciate that surfacing 🙏

[linkdotnet]

Sure, I will add both information

[linkdotnet]

Newest preview will have these information: https://www.nuget.org/packages/NCronJob/3.1.2-preview

[nulltoken]

Awesome! I’ll provide feedback tomorrow.

[nulltoken]

Plugged it. Works very nicely. Great reduction in the amount of boilerplate code.

Thanks a ton!