elsa-workflows / elsa-core

A .NET workflows library
https://v3.elsaworkflows.io/
MIT License
6.38k stars 1.18k forks source link

Hangfire Temporal Activities incorrect cron time for hours #4509

Open fshahin90 opened 1 year ago

fshahin90 commented 1 year ago

Hi, Im trying to create time out that will trigger after 1 hour so i added a Timer activity with durationFromSeconds(Number(getVariable("time_out"))) // time_out = 3600 i fount that the execution finishes imediatly within secounds like 2 to 5 secounds after execution.

when i tried to debug i found in the hangfire dashboard the cron expression for the job is * * */1 * * * thats generated from this file and i feel its wrong for multiple reasons:

  1. AFAIK cron only accepts 5 section
    • minutes
    • hours
    • day of month
    • month
    • day of week

there is no secounds as the class inserts

  1. to make the cron work for each hour the minute section should be (0 it will be at minute 0 from each hour ) not (* it will be every minute from each hour)

but if i tested with minutes like 30 minutes it works because the Hangfire cron library interprits the string from right to left excluding the excess note: i didnt test with the Quartz Library hope that every thing is clear

jdevillard commented 5 months ago

Hello,

I just started to use the schedule library and see the same issue. The code which translate TimeSpan to Cron seems to be wrong (you 2nd affirmation is correct , it should be 0 and not *)

https://github.com/elsa-workflows/elsa-core/blob/8289ae2cdfe54bfa848c1a89060de30ea44c33fc/src/modules/Elsa.Hangfire/Extensions/TimeSpanExtensions.cs#L13-L21

about your first remark, in .Net a lot of library are able to manipulate a Cron that use a six-part format to allow manipulation with second (NCronTab) .

You can check this lib for info : https://github.com/atifaziz/NCrontab and also this link to test your CRON expression : https://ncrontab.swimburger.net/

and it is also a little bit more difficult to write because :

So regarding the rule CreateCronComponent(int number) => (number > 0 ? $"*/{number}" : "*"); is not so easy and to convert a TimeSpan to a Cron Expression, we need to change this helper to take care of precedence.

For now, you could replace your Timer activity by a Cron to start a workflow until we deploy a fix to this activity (I don't have check Quartz, but this helper is only used in the Hangfire library)