Skip to content

fix: allow dynamic strings as jobSchedulerId in upsertJobScheduler (#3937) #3942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Manmeet567 wants to merge 2 commits into
base: master
Choose a base branch
from
+43 −1
Open

fix: allow dynamic strings as jobSchedulerId in upsertJobScheduler (#3937) #3942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14e449a
fix: allow dynamic strings as jobSchedulerId in upsertJobScheduler (#…
Manmeet567 Apr 6, 2026
f2ee341
test: add reproduction case for dynamic jobSchedulerId (#3937)
Manmeet567 Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/classes/queue.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,15 @@ export class Queue<
*
* @returns The next job to be scheduled (would normally be in delayed state).
*/
async upsertJobScheduler(
jobSchedulerId: string,
repeatOpts: Omit<RepeatOptions, 'key'>,
jobTemplate: {
name: NameType;
data?: DataType;
opts?: JobSchedulerTemplateOptions;
},
): Promise<Job<DataType, ResultType, NameType> | undefined>;
async upsertJobScheduler(
jobSchedulerId: NameType,
repeatOpts: Omit<RepeatOptions, 'key'>,
Expand All @@ -473,6 +482,15 @@ export class Queue<
data?: DataType;
opts?: JobSchedulerTemplateOptions;
},
): Promise<Job<DataType, ResultType, NameType> | undefined>;
async upsertJobScheduler(
jobSchedulerId: string,
repeatOpts: Omit<RepeatOptions, 'key'>,
jobTemplate?: {
name?: NameType;
data?: DataType;
opts?: JobSchedulerTemplateOptions;
},
) {
if (repeatOpts.endDate) {
if (+new Date(repeatOpts.endDate) < Date.now()) {
Expand All @@ -487,7 +505,7 @@ export class Queue<
>(
jobSchedulerId,
repeatOpts,
jobTemplate?.name ?? jobSchedulerId,
jobTemplate?.name ?? (jobSchedulerId as NameType),
jobTemplate?.data ?? <DataType>{},
{ ...this.jobsOpts, ...jobTemplate?.opts },
{ override: true },
Expand Down
24 changes: 24 additions & 0 deletions tests/test_issue_3937.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Queue } from '../src/classes/queue';
Copy link
Copy Markdown
Contributor

@manast manast Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of historical reasons we do not create files for specific issues, instead we add a new test on the proper test file that covers job schedulers functionality.

Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most tests import from the barrel export ../src/classes rather than deep-importing specific class files. Consider switching to import { Queue } from '../src/classes' to match the existing test conventions and avoid differences between entrypoints.

Suggested change
import { Queue } from '../src/classes/queue';
import { Queue } from '../src/classes';

Copilot uses AI. Check for mistakes.

type JobName = 'generate-report' | 'send-email';
Comment on lines +1 to +3
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file won’t actually protect against TypeScript regressions as written: tsconfig.json excludes tests/* from tsc, and Vitest transpiles TS without type-checking by default. To make this a real reproduction for #3937, consider adding a dedicated typecheck step for tests (e.g. vitest --typecheck or a tsc -p tsconfig.test.json script) or moving this into whatever type-test mechanism the repo uses.

Copilot uses AI. Check for mistakes.

interface JobData {
accountId: string;
}

const queue = new Queue<JobData, void, JobName>('issue-3937');
const dynamicId = String('inst_12345');

// Scheduler IDs must accept dynamic strings independently of the job name type.
queue.upsertJobScheduler(
dynamicId,
Comment on lines +9 to +14
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test file executes queue.upsertJobScheduler(...) at module top-level without awaiting the promise or cleaning up the Queue/Redis connection. In Vitest this will run during file import, can create open handles (hanging the run) and can trigger unhandled promise rejections. Please wrap this in an it(...) block (or an if (false) type-check-only block) and ensure the queue is closed/cleaned up if the call is actually executed.

Copilot uses AI. Check for mistakes.
{
every: 60_000,
},
{
name: 'generate-report',
data: {
accountId: 'acct_12345',
},
},
);