HangfireIO / HangfireIO/Hangfire

Wrong RecurringJob queue in version 1.8.3

Open
#2,249 0 comments 5 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
10.1k
Forks
1.8k
Avg merge
1h 19m
Merged PRs (30d)
1

Description

Hello,

In our solution we used HangFire 1.7.xx without problems, we recently updated to 1.8.3 and found out a problem with our recurring job queue management :

We used to create our recurring jobs using this signature :
`RecurringJob.AddOrUpdate(jodid, job => job.Execute(), "*/5 * * * *", new RecurringJobOptions { QueueName = "recurringQueue" });`
our jobs were created in the correct queue "recurringQueue" and correctly executed.

After switching to 1.8.3 version, `RecurringJobOptions.QueueName` is obsolete, so we tried to use the recommended signature instead :
`RecurringJob.AddOrUpdate(jodid, "recurringQueue", job => job.Execute(), "*/5 * * * *");`
with this signature, jobs are created in the "default" queue, even if we specified "recurringQueue". We had to roll back to use the previous signature.

By looking at the code behind something seems wrong:

The first signature we used using RecurringJobOptions :

```cs
public static void AddOrUpdate([NotNull] string recurringJobId, [InstantHandle][NotNull] Expression methodCall, [NotNull] Func cronExpression, [NotNull] RecurringJobOptions options)
{
if (cronExpression == null)
{
throw new ArgumentNullException("cronExpression");
}

AddOrUpdate(recurringJobId, methodCall, cronExpression(), options);
}
```

The second (recommended) :

```cs
public static void AddOrUpdate([NotNull] string recurringJobId, [NotNull] string queue, [InstantHandle][NotNull] Expression> methodCall, [NotNull] Func cronExpression)
{
AddOrUpdate(recurringJobId, queue, methodCall, cronExpression, new RecurringJobOptions());
}

```

Both ends up calling :

```cs
public static void AddOrUpdate([NotNull] string recurringJobId, [NotNull] string queue, [InstantHandle][NotNull] Expression> methodCall, [NotNull] Func cronExpression, [NotNull] RecurringJobOptions options)
{
if (cronExpression == null)
{
throw new ArgumentNullException("cronExpression");
}

AddOrUpdate(recurringJobId, queue, methodCall, cronExpression(), options);
}
```

which calls :

```cs
public static void AddOrUpdate([NotNull] string recurringJobId, [NotNull] string queue, [NotNull][InstantHandle] Expression> methodCall, [NotNull] string cronExpression, [NotNull] RecurringJobOptions options)
{
if (queue == null)
{
throw new ArgumentNullException("queue");
}

Job job = Job.FromExpression(methodCall, queue);
Instance.Value.AddOrUpdate(recurringJobId, job, cronExpression, options);
}
```

then :

```cs
public void AddOrUpdate(string recurringJobId, Job job, string cronExpression, RecurringJobOptions options)
{
if (recurringJobId == null)
{
throw new ArgumentNullException("recurringJobId");
}

if (job == null)
{
throw new ArgumentNullException("job");
}

if (cronExpression == null)
{
throw new ArgumentNullException("cronExpression");
}

if (options == null)
{
throw new ArgumentNullException("options");
}

ValidateCronExpression(cronExpression);
if (job.Queue != null && !Storage.HasFeature(JobStorageFeatures.JobQueueProperty))
{
throw new NotSupportedException("Current storage doesn't support specifying queues directly for a specific job. Please use the QueueAttribute instead.");
}

using IStorageConnection storageConnection = _storage.GetConnection();
using (storageConnection.AcquireDistributedRecurringJobLock(recurringJobId, DefaultTimeout))
{
RecurringJobEntity orCreateRecurringJob = storageConnection.GetOrCreateRecurringJob(recurringJobId, _timeZoneResolver, _nowFactory());
orCreateRecurringJob.Job = job;
orCreateRecurringJob.Cron = cronExpression;
orCreateRecurringJob.TimeZone = options.TimeZone;
orCreateRecurringJob.Queue = options.QueueName;
orCreateRecurringJob.MisfireHandling = options.MisfireHandling;
if (orCreateRecurringJob.IsChanged(out var changedFields, out var nextExecution))
{
using IWriteOnlyTransaction writeOnlyTransaction = storageConnection.CreateWriteTransaction();
writeOnlyTransaction.UpdateRecurringJob(orCreateRecurringJob, changedFields, nextExecution, _logger);
writeOnlyTransaction.Commit();
}
}
}
```

- When looking back at the first used method we can run through the code and see that the queue will be picked from RecurringJobOptions => orCreateRecurringJob.Queue = options.QueueName;, which is fine.

- But now if we begin running through the code using the second signature, we end up calling `AddOrUpdate(recurringJobId, queue, methodCall, cronExpression, new RecurringJobOptions());` which will set queue in the job object (`Job job = Job.FromExpression(methodCall, queue);)`, but will call the `RecurringJobOptions` constructor which set the queue to "default" and then the queue will also be picked from `RecurringJobOptions => orCreateRecurringJob.Queue = options.QueueName;` which is "default".
**In this scenario shouldn't the queue be acquiered from the job object in which we initialized our queue?**

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.