search

jobbrIO / jobbr-server

Jobbr is a non-invasive .NET JobServer
https://jobbr.readthedocs.io
GNU General Public License v3.0
50 stars 5 forks source link

Reenabling Trigger causes Process Crash #106

Closed eXpl0it3r closed 11 months ago

eXpl0it3r commented 1 year ago

Steps to Reproduce

  1. Create a reoccurring trigger
  2. Disable the trigger in the dashboard
  3. Reenable the trigger in the dashboard
  4. Wait for the next execution

Actual Behavior

The process crashes with the following exception:

Description: The process was terminated due to an unhandled exception.
Exception Info: System.ArgumentNullException: Value cannot be null. (Parameter 'expression')
   at NCrontab.CrontabSchedule.TryParse[T](String expression, ParseOptions options, Func`2 valueSelector, Func`2 errorSelector)
   at NCrontab.CrontabSchedule.Parse(String expression, ParseOptions options)
   at Jobbr.Server.Scheduling.Planer.RecurringJobRunPlaner.Plan(RecurringTrigger trigger)
   at Jobbr.Server.Scheduling.DefaultScheduler.EvaluateRecurringTriggers()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.TimerQueueTimer.Fire(Boolean isThreadPool)
   at System.Threading.TimerQueue.FireNextTimers()

Expected Behavior

The trigger successfully fires without server crash

eXpl0it3r commented 11 months ago

Root cause analysis:

When disabling the trigger on the Dashboard the WebApi is called, which das a converting from the web DTO to a trigger domain object. The DTO however doesn't contain all the information, e.g. the Definition isn't sent from the frontend to the backend, meaning the trigger information is overwritten with NULL in the DB and when the next scheduling tick, gets all the reoccurring triggers, it get a NULL value for the Cron Definition and crashes.

image

Looking at this, I also spotted that null as string (not NULL) is written into the parameters field, as it does a serializing of a C# null string.

https://github.com/jobbrIO/jobbr-webapi/blob/a1576b4a16dfc90c4626b6d8730bdb59a5346441/source/Jobbr.Server.WebAPI/Controller/TriggerController.cs#L76-L91

Potential fix: Use the saved trigger as basis and only apply changed/non-null values.

eXpl0it3r commented 11 months ago

Fixed by https://github.com/jobbrIO/jobbr-webapi/pull/37