diff --git a/.github/workflows/test-mongo-compatibility.yml b/.github/workflows/test-mongo-compatibility.yml new file mode 100644 index 0000000..f3e8d64 --- /dev/null +++ b/.github/workflows/test-mongo-compatibility.yml @@ -0,0 +1,84 @@ +name: Optional Mongo-compatible service tests + +on: + workflow_dispatch: + +jobs: + configured: + name: Check optional endpoint secrets + runs-on: ubuntu-latest + outputs: + cosmos: ${{ steps.check.outputs.cosmos }} + docdb: ${{ steps.check.outputs.docdb }} + steps: + - id: check + env: + COSMOS_MONGO_URL: ${{ secrets.COSMOS_MONGO_URL }} + DOCDB_URL: ${{ secrets.DOCDB_URL }} + run: | + if [ -n "$COSMOS_MONGO_URL" ]; then + echo 'cosmos=true' >> "$GITHUB_OUTPUT" + else + echo 'cosmos=false' >> "$GITHUB_OUTPUT" + echo '::notice::COSMOS_MONGO_URL is not configured; Cosmos test skipped.' + fi + if [ -n "$DOCDB_URL" ]; then + echo 'docdb=true' >> "$GITHUB_OUTPUT" + else + echo 'docdb=false' >> "$GITHUB_OUTPUT" + echo '::notice::DOCDB_URL is not configured; DocumentDB test skipped.' + fi + + test-cosmos: + name: Azure Cosmos DB Mongo API / mongodb@7 + if: needs.configured.outputs.cosmos == 'true' + needs: configured + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: npm + + - name: Install dependencies + run: npm ci || npm install + + - name: Pin MongoDB driver + run: npm install --no-save mongodb@7 + + - name: Mocha live tests (Cosmos DB Mongo API) + env: + MONGO_URL: ${{ secrets.COSMOS_MONGO_URL }} + run: npm run test:mongo + + test-documentdb: + name: AWS DocumentDB / mongodb@7 + if: needs.configured.outputs.docdb == 'true' + needs: configured + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: npm + + - name: Install dependencies + run: npm ci || npm install + + - name: Pin MongoDB driver + run: npm install --no-save mongodb@7 + + - name: Download AWS RDS CA bundle + run: curl -fsSLo "$RUNNER_TEMP/global-bundle.pem" https://truststore.pki.rds.amazonaws.com/global/global-bundle.pem + + - name: Mocha live tests (DocumentDB) + env: + MONGO_URL: ${{ secrets.DOCDB_URL }} + NODE_EXTRA_CA_CERTS: ${{ runner.temp }}/global-bundle.pem + run: npm run test:mongo diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8a892e9..8cf8f5d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,7 +13,7 @@ env: # pg@7 breaks on Node 14+ (readyState === 'open' before connect); engines require Node >= 20.9 LATEST_PG_DRIVER: '8' # Latest server images — literals in services.*.image and job name (env not allowed there) - # redis:8-alpine mongo:8 postgres:18-alpine + # redis:8-alpine mongo:8 postgres:18-alpine valkey/valkey:8.1.9-alpine eqalpha/keydb:latest jobs: # Types, adapter guards, Jest core + coverage (once, all services) @@ -324,6 +324,52 @@ jobs: REDIS_URL: redis://127.0.0.1:6379 run: npm run test:redis + # Redis-compatible engines × redis@5 (Node 22), standalone only + test-servers-redis-compatible: + name: ${{ matrix.engine }} / Node 22 / redis@5 + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - engine: Valkey 8.1.9 + image: valkey/valkey:8.1.9-alpine + - engine: KeyDB latest + image: eqalpha/keydb:latest + + services: + redis: + image: ${{ matrix.image }} + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping || valkey-cli ping || keydb-cli ping" + --health-interval 5s + --health-timeout 3s + --health-retries 10 + + steps: + - uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: npm + + - name: Install dependencies + run: npm ci || npm install + + - name: Pin redis@${{ env.LATEST_REDIS_DRIVER }} + run: | + npm install --no-save redis@${{ env.LATEST_REDIS_DRIVER }} + node -e "console.log('redis driver:', require('redis/package.json').version)" + + - name: Mocha live tests (Redis protocol) + env: + REDIS_URL: redis://127.0.0.1:6379 + run: npm run test:redis + # MongoDB server LTS+ lines (latest driver, Node 22) test-servers-mongo: name: Servers mongo:${{ matrix.mongo-server }} @@ -567,7 +613,7 @@ jobs: PG_URL: postgres://postgres:postgres@127.0.0.1:5432/josk_test run: bun test ./test/jest/adapters.test.js - # Meteor package tests — adapter suites only (METEOR_TEST_SUITE in package.js) + # Meteor package tests — adapter suites plus TypeScript runtime tests in one Redis cell # Mongo adapter: no mongo service — Meteor test-packages starts its bundled mongod when MONGO_URL is unset test-meteor-mongo: name: Meteor ${{ matrix.meteor }} / mongo adapter @@ -647,12 +693,18 @@ jobs: curl -fsSL "https://install.meteor.com?release=${{ matrix.meteor }}" | sh echo "$HOME/.meteor" >> $GITHUB_PATH - - name: Meteor package tests (Redis) + - name: Meteor default package suite (3.4 / Redis 8) + if: ${{ matrix.meteor == '3.4' && matrix.redis-server == '8' }} + env: + REDIS_URL: redis://127.0.0.1:6379 + run: meteor test-packages ./ --driver-package=meteortesting:mocha --once + + - name: Meteor package tests (Redis adapter) + if: ${{ matrix.meteor != '3.4' || matrix.redis-server != '8' }} env: METEOR_TEST_SUITE: redis REDIS_URL: redis://127.0.0.1:6379 - run: | - meteor test-packages ./ --driver-package=meteortesting:mocha --once + run: meteor test-packages ./ --driver-package=meteortesting:mocha --once test-meteor-postgres: name: Meteor ${{ matrix.meteor }} / pg:${{ matrix.postgres-server }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 00fc637..fc03a2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1 +1,8 @@ -See full change-log at [releases on GitHub](https://github.com/veliovgroup/josk/releases) \ No newline at end of file +## 6.4.0 + +- Suppress stale same-process interval updates, retain superseded handlers during shutdown, and clarify custom adapter fencing requirements. +- Report unfinished shutdown handlers, hand interval claims back, and abandon one-shots to preserve at-most-once execution. +- Add standalone KeyDB and Valkey CI smoke tests; clarify Mongo-compatible service coverage. +- Exercise TypeScript runtime tests through `meteor test-packages`; type-check all adapter contracts and `shutdown()` through the Meteor import with `tsc`. + +See full change-log at [releases on GitHub](https://github.com/veliovgroup/josk/releases) diff --git a/README.md b/README.md index 1a997ee..217542c 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ __Note: JoSk is the server-only package.__ ## Prerequisites - `node@>=20.9.0` -- `redis-server@>=5.0.0`, KeyDB, or Valkey for `RedisAdapter`, with the `redis@^4` or `redis@^5` NPM package +- `redis-server@>=5.0.0` or a single-writer KeyDB/Valkey server for `RedisAdapter`, with `redis@^4` or `redis@^5`. CI targets selected standalone images; see [Redis Adapter](#redis-adapter) for exact coverage. - `mongod@>=4.4` for `MongoAdapter`, with the official `mongodb` NPM package (only the official driver is tested) - `postgres@>=12` for `PostgresAdapter`, with `pg@>=8.0.3` (`pg@7` does not connect on Node 14+) - `bun@>=1.1.0` (optional), see [Bun runtime](#bun-runtime) @@ -175,10 +175,10 @@ JoSk has no dependencies. Install the driver for your adapter: `redis`, `mongodb #### Redis Adapter -`RedisAdapter` keeps due timestamps in a sorted set and task payloads in a hash, and claims due tasks with Lua scripts. It works with Redis, [KeyDB](https://docs.keydb.dev/), and Valkey. +`RedisAdapter` keeps due timestamps in a sorted set and task payloads in a hash, and claims due tasks with Lua scripts. It uses the Redis-compatible commands shared by Redis, [KeyDB](https://docs.keydb.dev/), and Valkey; CI targets Redis 6/7/8, `eqalpha/keydb:latest`, and `valkey/valkey:8.1.9-alpine` in standalone mode using `redis@5`. - Use one writable primary. Do not route JoSk reads or writes to replicas; claims must be visible to all instances at once. -- For Redis Cluster or KeyDB Cluster, pass `useHashTags: true`. +- For Redis Cluster, pass `useHashTags: true`; CI tests a 3-master Redis Cluster. KeyDB/Valkey Cluster modes are not separately tested. - Avoid KeyDB active-replication (multi-master). Its conflict resolution can let two writers claim the same task. - For strict single-claim scheduling across data centers, use a strongly consistent store, or PostgreSQL with one write authority. @@ -194,7 +194,7 @@ const jobs = new JoSk({ adapter: new RedisAdapter({ client: redisClient, prefix: 'app-scheduler', - // useHashTags: true, // Enable for Redis Cluster / KeyDB Cluster + // useHashTags: true, // Redis Cluster; KeyDB/Valkey Cluster not CI-tested }), onError(reason, details) { // Catches exceptions thrown inside scheduled tasks @@ -418,7 +418,7 @@ Stops this instance's scheduler. After `destroy()`, only `clearTimeout()` and `c - `opts.timeout` {*Number*} - [Optional] Milliseconds to wait for running handlers to call `ready()`. Default: `10000` - Returns: {*`Promise`*} `true` if every running handler finished within `timeout` -Calls `destroy()`, waits for running handlers, then hands unfinished interval claims back to storage so another instance runs them on its next poll instead of after `zombieTime`. A handler that calls `ready()` after its claim was handed back does not change the schedule. Keep `timeout` below your platform's termination grace period. +Calls `destroy()`, waits for running handlers, then hands unfinished interval claims back to storage so another instance runs them on its next poll instead of after `zombieTime`. A handler that calls `ready()` after its claim was handed back does not change the schedule. Repeated calls share the first shutdown attempt and timeout; when it expires, JoSk reports every unfinished handler to `onError` (or `console.error`), hands current interval claims back, and abandons one-shot tasks to preserve at-most-once behavior. Superseded handlers also count as unfinished, but their obsolete claims are left unchanged. Keep at-most-once handlers idempotent, or set `timeout` longer than the longest one-shot handler; keep it below your platform's termination grace period. ```js const shutdown = async () => { @@ -481,7 +481,7 @@ Queue-polling pattern: after winning a tick, claim rows from your own queue, cal | `setTimeout`, `setImmediate` | **At-most-once** across the cluster | Task is removed from storage *before* the handler runs. If the process dies mid-run, the run is lost. | | `setInterval` | **At-least-once** per tick (until cleared) | Task stays in storage while running. If `ready()` is not called within `zombieTime`, another instance may claim and run it again. Make handlers idempotent. | -`zombieTime` (default 15 minutes) is the safety net for stuck handlers. Set it above your slowest legitimate handler plus storage latency. Restarting an app does not shorten a running interval's hold. So after an unclean kill, recovery waits the full `zombieTime`, not `delay`, but a rolling deploy cannot start a second copy of a handler that is still running. Call [`shutdown()`](#shutdownopts) on `SIGTERM` to hand running claims back instead of leaving them to `zombieTime`. A handler that finishes after its claim was recovered cannot overwrite the newer run's schedule. See [monitoring and recovery](docs/monitoring.md). +`zombieTime` (default 15 minutes) is the safety net for stuck handlers. Set it above your slowest legitimate handler plus storage latency. Restarting an app does not shorten a running interval's hold. So after an unclean kill, recovery waits the full `zombieTime`, not `delay`, but a rolling deploy cannot start a second copy of a handler that is still running. Call [`shutdown()`](#shutdownopts) on `SIGTERM` to hand running claims back instead of leaving them to `zombieTime`. JoSk skips a late `ready()` write when a newer same-uid run starts in the same process. Built-in adapters also fence writes across instances; custom adapters need equivalent fencing for that guarantee. See [adapter-api.md](docs/adapter-api.md) and [monitoring and recovery](docs/monitoring.md). - `execute: 'batch'` (default) claims all due tasks under one lease, for throughput. `execute: 'one'` claims one task per lease, for smaller bursts and fairer spread across instances. - `concurrency` caps parallel handlers in this instance. Default `Infinity`, like Node's timers. Set a limit when handlers share DB connections or API rate limits. @@ -589,7 +589,7 @@ DELETE FROM josk_locks WHERE lock_key = 'josk-cluster-scheduler.lock'; ### MongoDB connection fine tuning -Replica-set tuning, dedicated-DB advice, the index list, and Mongoose / CosmosDB / DocumentDB notes: [`docs/mongodb.md`](docs/mongodb.md). +Replica-set tuning, dedicated-DB advice, the index list, and Mongo-compatible service notes: [`docs/mongodb.md`](docs/mongodb.md). Cosmos DB and DocumentDB aren't part of default CI; optional endpoint tests require configured secrets. ## Prefix mapping diff --git a/adapters/blank-example.d.ts b/adapters/blank-example.d.ts index 7dd97ec..3dbc6a9 100644 --- a/adapters/blank-example.d.ts +++ b/adapters/blank-example.d.ts @@ -79,7 +79,8 @@ export class BlankAdapter { /** * @async * @memberOf BlankAdapter - * Update next execution timestamp + * Update next execution timestamp. Apply the lease filter, schedule write, and + * claimLeaseId clear atomically; without a lease, preserve unconditional updates. * @name update * @param {object} task - Full object of task from storage * @param {Date} nextExecuteAt - Date defining time of next execution @@ -89,7 +90,7 @@ export class BlankAdapter { /** * @async * @memberOf BlankAdapter - * Claim due tasks atomically and execute them + * Claim due tasks atomically, persist lock.leaseId as claimLeaseId, and execute them * @name iterate * @param {Date} nextExecuteAt - Date defining time of next execution for zombie recovery * @param {{ ownerId: string, leaseId: string }} lock diff --git a/adapters/blank-example.js b/adapters/blank-example.js index 088ebe2..268eaff 100644 --- a/adapters/blank-example.js +++ b/adapters/blank-example.js @@ -158,7 +158,8 @@ class BlankAdapter { /** * @async * @memberOf BlankAdapter - * Update next execution timestamp + * Update next execution timestamp. Apply the lease filter, schedule write, and + * claimLeaseId clear atomically; without a lease, preserve unconditional updates. * @name update * @param {object} task - Full object of task from storage * @param {Date} nextExecuteAt - Date defining time of next execution @@ -178,10 +179,18 @@ class BlankAdapter { await this.ready(); try { - return await this.requiredOption.update({ + const filter = { scope: this.uniqueName, - uid: task.uid, - executeAt: +nextExecuteAt + uid: task.uid + }; + if (typeof task.claimLeaseId === 'string' && task.claimLeaseId !== '') { + filter.claimLeaseId = task.claimLeaseId; + } + + return await this.requiredOption.update({ + filter, + set: { executeAt: +nextExecuteAt }, + unset: ['claimLeaseId'] }); } catch (opError) { this.joskInstance.__errorHandler(opError, '[StorageAdapter] [update] [opError]', 'Exception inside StorageAdapter#update() method', task.uid); @@ -192,7 +201,7 @@ class BlankAdapter { /** * @async * @memberOf BlankAdapter - * Claim due tasks atomically and execute them + * Claim due tasks atomically, persist lock.leaseId as claimLeaseId, and execute them * @name iterate * @param {Date} nextExecuteAt - Date defining time of next execution for zombie recovery * @param {{ ownerId: string, leaseId: string }} lock @@ -219,7 +228,7 @@ class BlankAdapter { } executed++; - this.joskInstance.__execute(task); + this.joskInstance.__execute({ ...task, claimLeaseId: lock.leaseId }); } return executed; diff --git a/docs/adapter-api.md b/docs/adapter-api.md index 2fd4e01..2e94ee6 100644 --- a/docs/adapter-api.md +++ b/docs/adapter-api.md @@ -11,7 +11,7 @@ Start from [`blank-example.js`](https://github.com/veliovgroup/josk/blob/master/ - Keep second-layer scheduler lock. Use owner-bound lease token. Never release foreign lease. - Derive lock lifetime from the lock object itself — prefer the relative `lock.leaseMs`; use `lock.expireAt` / `lock.expiresAtMs` only as a fallback for locks minted without it. Never substitute `zombieTime` (a substituted long TTL freezes the whole prefix for up to `zombieTime` when a holder dies uncleanly), and never re-derive a duration as `expiresAtMs - Date.now()` when `leaseMs` is present — that second app-clock read is distorted by any clock step between mint and acquire. - Claim due tasks atomically in storage. Do not `find all due -> update later`. -- Fence `update()` on the claim: return `claimLeaseId: lock.leaseId` on claimed tasks, and when `update()` receives a task with `claimLeaseId`, write only if the stored lease still matches, then clear it. Otherwise a handler that finishes after zombie recovery overwrites the newer run's schedule. A task without `claimLeaseId` updates unconditionally. JoSk also calls `update(task, now)` to hand a claim back on `destroy()` / `shutdown()`. +- Fence `update()` on the claim: atomically store and return `claimLeaseId: lock.leaseId`; `update()` must match that lease, update the schedule, and clear the lease in one write. Without a token, update unconditionally for compatibility. JoSk also suppresses a superseded run's late `ready()` write when a newer same-uid run starts in this process; cross-instance safety still needs adapter fencing. JoSk calls `update(task, now)` to hand a claim back on `destroy()` / `shutdown()`. - `iterate()` should claim and execute `one` or `batch` depending on `executeMode`. - `ready()` optional but recommended. Use it to finish schema/index/init work before first storage op. - Prefer storage-server time over client time when comparing lease expirations. Mixed client clocks across a cluster will cause incorrect lock ownership otherwise. See `adapters/postgres.js` (`CURRENT_TIMESTAMP` in `acquireLock`) for a reference pattern. @@ -79,4 +79,21 @@ Inside `Adapter#iterate()` call `this.joskInstance.__execute(task)` with: 4. Call `this.joskInstance.__execute(task)`. 5. Release scheduler lease only if owner token still matches. +For example, atomically persist and return the claim token: + +```js +return { ...task, claimLeaseId: lock.leaseId }; +``` + +A Mongo-style atomic `update()` filters on the token when present, updates the schedule, and clears it. Without a token, it filters by task identity only: + +```js +const filter = { uid: task.uid }; +if (task.claimLeaseId) filter.claimLeaseId = task.claimLeaseId; +await collection.updateOne(filter, { + $set: { executeAt: nextExecuteAt }, + $unset: { claimLeaseId: '' } +}); +``` + Global lock alone is not enough for duplicate prevention. Atomic task claim is required. diff --git a/docs/meteor.md b/docs/meteor.md index 205c4c0..ff578fd 100644 --- a/docs/meteor.md +++ b/docs/meteor.md @@ -1,6 +1,6 @@ # JoSk usage within Meteor.js -NPM `josk` package can be used in Meteor environment just perfectly fine since it's server-only Node.js package. +The NPM `josk` package can run in Meteor server code. CI exercises Meteor 2.14–2.16 and 3.2/3.3.1/3.4 with adapter-specific package suites; see [test coverage](#requirements). If Meteor.js packages are preferred in your project/environment follow this document to install JoSk as `ostrio:cron-jobs` [Atmosphere](https://atmospherejs.com/ostrio/cron-jobs) or [Packosphere](https://packosphere.com/ostrio/cron-jobs) package @@ -100,7 +100,7 @@ const jobs = new JoSk({ }); ``` -Use one writable Redis/KeyDB primary. Do not route JoSk traffic to replicas. For Redis Cluster / KeyDB Cluster, set `useHashTags: true`. +Use one writable Redis-compatible primary. Do not route JoSk traffic to replicas. For Redis Cluster, set `useHashTags: true`; KeyDB/Valkey Cluster also require it but are not covered by cluster CI. Meteor CI tests Redis 6/7/8 only; KeyDB and Valkey are not tested within Meteor. ### PostgreSQL Adapter @@ -233,7 +233,7 @@ Meteor **2.14+** and **3.2+** supported (`api.versionsFrom(['2.14', '3.2'])`; mi | 14–17 | 2.x | `chai@4`, `cron-parser@4`, `pg@8.11` | `meteortesting:mocha@2.1.0` | | 18+ | 3.x | current majors | `meteortesting:mocha@3.3.0` | -TypeScript tests (`meteor-types.ts`) always run. npm `devDependencies` unchanged for `npm test`. +The default local suite includes `test/meteor-types.ts`. CI runs adapter-specific `meteor test-packages` suites for Meteor 2.14–2.16 and 3.2/3.3.1/3.4; one Meteor 3.4 / Redis 8 cell runs the default suite, including the TypeScript runtime tests and `shutdown()`. Meteor transpiles TypeScript without type-checking. Separately, `npm run test:types` runs `tsc` against the packaged Meteor declaration assets to check all built-in adapter contracts and the `shutdown()` signature through `meteor/ostrio:cron-jobs`. npm `devDependencies` stay unchanged for `npm test`. Do not commit `.versions` — it locks packages for one Meteor release and breaks multi-version CI. Pin the driver in `package.js` only; pass `--driver-package=meteortesting:mocha` on the CLI (`@x.y.z` on the CLI breaks `test-packages` on Meteor 3.x): diff --git a/docs/migration-v6-v6.1.md b/docs/migration-v6-v6.1.md index 4c75215..53886a9 100644 --- a/docs/migration-v6-v6.1.md +++ b/docs/migration-v6-v6.1.md @@ -16,7 +16,7 @@ If you ran v5/v6.0 on a Redis Cluster or KeyDB Cluster: opt into `useHashTags: t ## When to enable `useHashTags` -Use `useHashTags: true` when JoSk runs against **Redis Cluster** or **KeyDB Cluster**, so Lua scripts that touch `schedule`, `tasks`, and `lock` keys stay in one hash slot. Standalone Redis / single-primary KeyDB can keep the default (`false`). +Use `useHashTags: true` when JoSk runs against Redis Cluster so Lua scripts that touch `schedule`, `tasks`, and `lock` keys stay in one hash slot. The same requirement applies to KeyDB/Valkey Cluster, but CI cluster coverage uses Redis only. Standalone Redis-compatible servers can keep the default (`false`). ## Key migration diff --git a/docs/migration-v6.3-v6.4.md b/docs/migration-v6.3-v6.4.md index e068838..9857f02 100644 --- a/docs/migration-v6.3-v6.4.md +++ b/docs/migration-v6.3-v6.4.md @@ -23,11 +23,13 @@ process.on('SIGTERM', async () => { }); ``` -`destroy()` still returns immediately. It now hands back tasks this instance claimed but had not started. +`destroy()` still returns immediately. It now hands back tasks this instance claimed but had not started. Repeated `shutdown()` calls share the first attempt and timeout. If it times out, unfinished runs are reported through `onError` (or `console.error`); interval claims are handed back, while one-shots are abandoned to preserve at-most-once behavior. Keep one-shot handlers idempotent, or set `timeout` above their longest runtime. ## Stale `ready()` calls -A handler that calls `ready()` after its claim was recovered by another instance no longer overwrites the newer run's schedule. +JoSk skips a late `ready()` write when a newer same-uid run starts in the same process. Built-in adapters also fence writes after recovery by another instance. Custom adapters without claim-lease or equivalent update fencing remain compatible, but a late handler on another process can overwrite the newer schedule. + +Shutdown still waits for superseded handlers to finish. If one remains unfinished at timeout, shutdown returns `false`, but does not hand its obsolete claim back or change the newer run's schedule. ## Rollout @@ -45,4 +47,4 @@ Declarations no longer import `redis` or `mongodb`, so projects with `skipLibChe ## Custom adapters -Optional, recommended: return `claimLeaseId: lock.leaseId` on claimed tasks and make `update()` write only while the stored lease matches, then clear it. See [adapter-api.md](adapter-api.md). Adapters that ignore the field keep working without fencing. +Optional, recommended: return `claimLeaseId: lock.leaseId` on claimed tasks and make `update()` match the stored lease, update the schedule, and clear it atomically. JoSk already suppresses late updates from superseded same-process runs. Adapters that ignore the field keep working, but need equivalent storage fencing for cross-instance protection. See [adapter-api.md](adapter-api.md). diff --git a/docs/mongodb.md b/docs/mongodb.md index eb1a799..7380dac 100644 --- a/docs/mongodb.md +++ b/docs/mongodb.md @@ -64,4 +64,6 @@ Do not drop or modify these indexes manually — `__setup` recreates them on nex ## Mongoose, CosmosDB, DocumentDB -`MongoAdapter` is **tested against the official `mongodb` driver only**. Other clients may work if they expose the same `Db.collection()`, `Db.command()`, and `Collection` APIs, but are unsupported. CosmosDB's Mongo API in particular omits operators the scheduler relies on for atomic claim semantics. +`MongoAdapter` default CI tests the official driver: `mongodb@5/6/7` with `mongo:8`, and `mongodb@7` with `mongo:6/7/8`. Cosmos DB for MongoDB and Amazon DocumentDB are not part of default CI. The manual [Mongo compatibility workflow](../.github/workflows/test-mongo-compatibility.yml) runs `npm run test:mongo` against `COSMOS_MONGO_URL` or `DOCDB_URL` only when that secret is configured; a passing run covers this test suite and endpoint, not every Mongo API feature or service version. Each secret must include the database path and the target service's required connection options; the DocumentDB job adds AWS's global CA bundle. + +Microsoft's [current Linux emulator vNext](https://learn.microsoft.com/en-us/azure/cosmos-db/emulator-linux) supports only the NoSQL API. Microsoft [release notes](https://learn.microsoft.com/en-us/azure/cosmos-db/emulator-release-notes) also document local MongoDB endpoints through API 4.2, including a legacy Linux Docker endpoint; that limited emulator support does not establish parity with current cloud Cosmos Mongo API versions. AWS describes DocumentDB as a managed VPC service and publishes a versioned [MongoDB compatibility guide](https://docs.aws.amazon.com/documentdb/latest/devguide/compatibility.html); no AWS-provided local emulator was identified for CI. Cosmos/DocumentDB support remains endpoint- and version-specific. The adapter requires atomic `findOneAndUpdate` with sort/return-before, batched `bulkWrite` claims, update-pipeline interval upserts, and TTL indexes. Verify these operations against the target API; keep the service unverified until its optional workflow passes. Mongoose wrappers remain untested and unsupported. diff --git a/index.cjs b/index.cjs index fa064d4..bab7e9e 100644 --- a/index.cjs +++ b/index.cjs @@ -2041,8 +2041,14 @@ class JoSk { this.__pausedAll = false; /** @internal @type {Set} */ this.__pausedTimerIds = new Set(); - /** @internal @type {Map void, done: Promise }>} */ + /** @internal @type {Map void, done: Promise }>} */ this.__inFlight = new Map(); + /** @internal @type {Promise | null} */ + this.__shutdownPromise = null; + /** @internal @type {Set<{ task: JoSkTask, released: boolean, superseded: boolean, settle: () => void, done: Promise }>} */ + this.__running = new Set(); + /** @internal */ + this.__warnedMissingClaimLeaseId = false; /** @internal @type {JoSkTask[]} */ this.__releaseQueue = []; /** @internal @type {Set>} */ @@ -2263,7 +2269,8 @@ class JoSk { * @memberOf JoSk * Destroy the instance, wait for running handlers to call `ready()`, then * hand unfinished interval claims back to storage so another instance can - * run them without waiting for `zombieTime`. Call before process exit. + * run them without waiting for `zombieTime`. Report unfinished runs at timeout; + * abandon one-shot tasks to preserve at-most-once execution. Call before exit. * @name shutdown * @param {JoSkShutdownOption} [opts] * @returns {Promise} - `true` if every running handler finished within `timeout` @@ -2274,28 +2281,51 @@ class JoSk { throw new Error(errors.shutdownTimeout); } + if (!this.__shutdownPromise) { + this.__shutdownPromise = this.__shutdown(timeout); + } + + return await this.__shutdownPromise; + } + + /** @internal */ + async __shutdown(timeout) { this.destroy(); if (this.__iteratePromise) { await this.__iteratePromise; } - if (this.__inFlight.size > 0) { + if (this.__running.size > 0) { let timer; const timedOut = new Promise((resolve) => { timer = setTimeout(resolve, timeout); }); - await Promise.race([Promise.all([...this.__inFlight.values()].map((run) => run.done)), timedOut]); + await Promise.race([Promise.all([...this.__running].map((run) => run.done)), timedOut]); clearTimeout(timer); } - const unfinished = [...this.__inFlight.values()]; + const unfinished = [...this.__running]; for (const run of unfinished) { run.released = true; - this.__inFlight.delete(run.task.uid); - run.settle(); - if (run.task.isInterval === true) { + this.__finishRun(run); + + const isInterval = run.task.isInterval === true; + const kind = isInterval ? 'interval' : 'one-shot'; + let outcome = 'abandoned to preserve at-most-once execution'; + if (isInterval) { + outcome = run.superseded + ? 'superseded claim left unchanged' + : 'claim queued to be handed back to storage'; + } + if (isInterval && !run.superseded) { this.__queueRelease(run.task); } + this.__errorHandler( + new Error(`${kind} task ${run.task.uid} did not finish within shutdown timeout; ${outcome}`), + '[shutdown] timeout', + `Unfinished ${kind} task; ${outcome}`, + run.task.uid + ); } this.__flushReleases(); @@ -2623,6 +2653,11 @@ class JoSk { return; } + if (task.isInterval === true && (typeof task.claimLeaseId !== 'string' || task.claimLeaseId === '') && !this.__warnedMissingClaimLeaseId && this.debug === true) { + this.__warnedMissingClaimLeaseId = true; + this._debug(`[${task.uid}] [__execute] claimed task has no claimLeaseId; late ready() calls can overwrite a newer cross-instance run unless adapter fences updates another way`); + } + if (this.__isTaskPaused(task.uid)) { await this.__deferClaimedTask(task); return; @@ -2655,7 +2690,7 @@ class JoSk { } try { - if (task.isInterval === true && !run.released) { + if (task.isInterval === true && !run.released && !run.superseded) { let nextExecuteAt = new Date(timestamp + task.delay); if (typeof readyArg1 === 'object' && readyArg1 instanceof Date && +readyArg1 >= timestamp) { nextExecuteAt = readyArg1; @@ -2776,27 +2811,28 @@ class JoSk { __trackRun(task) { const previous = this.__inFlight.get(task.uid); if (previous) { - this.__inFlight.delete(task.uid); - previous.settle(); + previous.superseded = true; } let settle = () => {}; const done = new Promise((resolve) => { settle = resolve; }); - const run = { task, released: false, settle, done }; + const run = { task, released: false, superseded: false, settle, done }; this.__inFlight.set(task.uid, run); + this.__running.add(run); return run; } /** * @internal - * @param {{ task: JoSkTask, settle: () => void }} run + * @param {{ task: JoSkTask, released: boolean, superseded: boolean, settle: () => void, done: Promise }} run */ __finishRun(run) { if (this.__inFlight.get(run.task.uid) === run) { this.__inFlight.delete(run.task.uid); } + this.__running.delete(run); run.settle(); } diff --git a/index.d.ts b/index.d.ts index c1e67ab..24309df 100644 --- a/index.d.ts +++ b/index.d.ts @@ -177,7 +177,8 @@ export class JoSk { * @memberOf JoSk * Destroy the instance, wait for running handlers to call `ready()`, then * hand unfinished interval claims back to storage so another instance can - * run them without waiting for `zombieTime`. Call before process exit. + * run them without waiting for `zombieTime`. Report unfinished runs at timeout; + * abandon one-shot tasks to preserve at-most-once execution. Call before exit. * @name shutdown * @param {JoSkShutdownOption} [opts] * @returns {Promise} - `true` if every running handler finished within `timeout` diff --git a/index.js b/index.js index c8c33e8..925c4c0 100644 --- a/index.js +++ b/index.js @@ -198,8 +198,14 @@ class JoSk { this.__pausedAll = false; /** @internal @type {Set} */ this.__pausedTimerIds = new Set(); - /** @internal @type {Map void, done: Promise }>} */ + /** @internal @type {Map void, done: Promise }>} */ this.__inFlight = new Map(); + /** @internal @type {Promise | null} */ + this.__shutdownPromise = null; + /** @internal @type {Set<{ task: JoSkTask, released: boolean, superseded: boolean, settle: () => void, done: Promise }>} */ + this.__running = new Set(); + /** @internal */ + this.__warnedMissingClaimLeaseId = false; /** @internal @type {JoSkTask[]} */ this.__releaseQueue = []; /** @internal @type {Set>} */ @@ -420,7 +426,8 @@ class JoSk { * @memberOf JoSk * Destroy the instance, wait for running handlers to call `ready()`, then * hand unfinished interval claims back to storage so another instance can - * run them without waiting for `zombieTime`. Call before process exit. + * run them without waiting for `zombieTime`. Report unfinished runs at timeout; + * abandon one-shot tasks to preserve at-most-once execution. Call before exit. * @name shutdown * @param {JoSkShutdownOption} [opts] * @returns {Promise} - `true` if every running handler finished within `timeout` @@ -431,28 +438,51 @@ class JoSk { throw new Error(errors.shutdownTimeout); } + if (!this.__shutdownPromise) { + this.__shutdownPromise = this.__shutdown(timeout); + } + + return await this.__shutdownPromise; + } + + /** @internal */ + async __shutdown(timeout) { this.destroy(); if (this.__iteratePromise) { await this.__iteratePromise; } - if (this.__inFlight.size > 0) { + if (this.__running.size > 0) { let timer; const timedOut = new Promise((resolve) => { timer = setTimeout(resolve, timeout); }); - await Promise.race([Promise.all([...this.__inFlight.values()].map((run) => run.done)), timedOut]); + await Promise.race([Promise.all([...this.__running].map((run) => run.done)), timedOut]); clearTimeout(timer); } - const unfinished = [...this.__inFlight.values()]; + const unfinished = [...this.__running]; for (const run of unfinished) { run.released = true; - this.__inFlight.delete(run.task.uid); - run.settle(); - if (run.task.isInterval === true) { + this.__finishRun(run); + + const isInterval = run.task.isInterval === true; + const kind = isInterval ? 'interval' : 'one-shot'; + let outcome = 'abandoned to preserve at-most-once execution'; + if (isInterval) { + outcome = run.superseded + ? 'superseded claim left unchanged' + : 'claim queued to be handed back to storage'; + } + if (isInterval && !run.superseded) { this.__queueRelease(run.task); } + this.__errorHandler( + new Error(`${kind} task ${run.task.uid} did not finish within shutdown timeout; ${outcome}`), + '[shutdown] timeout', + `Unfinished ${kind} task; ${outcome}`, + run.task.uid + ); } this.__flushReleases(); @@ -780,6 +810,11 @@ class JoSk { return; } + if (task.isInterval === true && (typeof task.claimLeaseId !== 'string' || task.claimLeaseId === '') && !this.__warnedMissingClaimLeaseId && this.debug === true) { + this.__warnedMissingClaimLeaseId = true; + this._debug(`[${task.uid}] [__execute] claimed task has no claimLeaseId; late ready() calls can overwrite a newer cross-instance run unless adapter fences updates another way`); + } + if (this.__isTaskPaused(task.uid)) { await this.__deferClaimedTask(task); return; @@ -812,7 +847,7 @@ class JoSk { } try { - if (task.isInterval === true && !run.released) { + if (task.isInterval === true && !run.released && !run.superseded) { let nextExecuteAt = new Date(timestamp + task.delay); if (typeof readyArg1 === 'object' && readyArg1 instanceof Date && +readyArg1 >= timestamp) { nextExecuteAt = readyArg1; @@ -933,27 +968,28 @@ class JoSk { __trackRun(task) { const previous = this.__inFlight.get(task.uid); if (previous) { - this.__inFlight.delete(task.uid); - previous.settle(); + previous.superseded = true; } let settle = () => {}; const done = new Promise((resolve) => { settle = resolve; }); - const run = { task, released: false, settle, done }; + const run = { task, released: false, superseded: false, settle, done }; this.__inFlight.set(task.uid, run); + this.__running.add(run); return run; } /** * @internal - * @param {{ task: JoSkTask, settle: () => void }} run + * @param {{ task: JoSkTask, released: boolean, superseded: boolean, settle: () => void, done: Promise }} run */ __finishRun(run) { if (this.__inFlight.get(run.task.uid) === run) { this.__inFlight.delete(run.task.uid); } + this.__running.delete(run); run.settle(); } diff --git a/package.json b/package.json index 0bdc0df..91c91b9 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "test:coverage": "node --experimental-vm-modules ./node_modules/jest/bin/jest.js --config jest.config.mjs --coverage", "test:types": "tsc -p ./tsconfig.types.json && node ./test/types/packed.mjs", "test:redis": "mocha ./test/npm-redis.js", - "test:mongo": "mocha ./test/npm-mongo.js", + "test:mongo": "mocha ./test/npm-mongo.js ./test/mongo-url.test.js", "test:postgres": "mocha ./test/npm-postgres.js" }, "keywords": [ diff --git a/skills/josk/references/adapters.md b/skills/josk/references/adapters.md index 6c89879..642861c 100644 --- a/skills/josk/references/adapters.md +++ b/skills/josk/references/adapters.md @@ -59,15 +59,15 @@ The `{app}` braces are Redis hash tags that keep all adapter keys on the same Cl ### Engines: Redis, KeyDB, Valkey -Same RESP client (`redis@^4 \|\| ^5`). JoSk is Lua-always (HASH + ZSET + `SET NX PX` + `cjson`). No `WATCH`. No RedisJSON / Streams / modules. +Same RESP client (`redis@^4 \|\| ^5`). JoSk is Lua-always (HASH + ZSET + `SET NX PX` + `cjson`). No `WATCH`. No RedisJSON / Streams / modules. `Proven` lists exact CI targets, not all-version guarantees. | Engine | Standalone | Cluster | Proven | |---|---|---|---| -| Redis | Yes | `useHashTags: true` | CI: Redis 6 / 7 / 8 standalone. No Cluster job | -| KeyDB | Yes, as single-writer Redis | `useHashTags: true` | README: well-tested. No CI image | -| Valkey | Yes (Redis 7.2 fork) | `useHashTags: true` | Not in CI; treat as Redis 7 | +| Redis | Yes | `useHashTags: true` | CI targets Redis 6/7/8 standalone and a 3-master Redis Cluster (drivers 4/5) | +| KeyDB | Yes, as single-writer Redis | `useHashTags: true` | CI targets `eqalpha/keydb:latest` standalone (Node 22, `redis@5`); no KeyDB Cluster job | +| Valkey | Yes (Redis-compatible) | `useHashTags: true` | CI targets `valkey/valkey:8.1.9-alpine` standalone (Node 22, `redis@5`); no Valkey Cluster job | -**Will run well:** one writable primary, or Cluster with `useHashTags: true`. JoSk itself is fine on Redis ≥ 5. +**Topology guidance:** use one writable primary. Redis-compatible Cluster requires `useHashTags: true`; CI Cluster coverage uses Redis only. JoSk itself is fine on Redis ≥ 5. **Will not:** KeyDB active-replication / multi-master; Redis active-active (CRDT); replica reads; Cluster without `useHashTags` (`CROSSSLOT` on Lua). Multi-DC / mixed clocks → `PostgresAdapter`. @@ -139,7 +139,7 @@ const options = { const client = await MongoClient.connect('mongodb://…', options); ``` -`MongoAdapter` is verified against the official `mongodb` NPM package only. CosmosDB, DocumentDB, Mongoose wrappers — flag as untested when recommending. +`MongoAdapter` default CI tests official `mongodb@5/6/7` with `mongo:8`, plus `mongodb@7` with `mongo:6/7/8`. Cosmos DB for MongoDB and DocumentDB are excluded from default CI; the manual workflow tests configured endpoints only when secrets are present. Mongoose wrappers remain untested. Treat each cloud service/API version as unverified until its optional test passes. ## `PostgresAdapter` diff --git a/skills/josk/references/api.md b/skills/josk/references/api.md index 3fe8ae7..a7ee361 100644 --- a/skills/josk/references/api.md +++ b/skills/josk/references/api.md @@ -116,7 +116,7 @@ Stops the internal revolving timer. Returns `true` the first time, `false` on su ### `shutdown({ timeout? })` → `Promise` -Since 6.4.0. Calls `destroy()`, waits up to `timeout` ms (default `10000`) for running handlers to call `ready()`, then hands unfinished interval claims back to storage (`executeAt = now`) so another instance runs them on its next poll, not after `zombieTime`. Resolves `true` if every handler finished in time. A late `ready()` from a handed-back run does not touch the schedule. Rejects on a negative or non-finite `timeout`. Keep `timeout` below the platform grace period (Kubernetes default 30s). +Since 6.4.0. Calls `destroy()`, waits up to `timeout` ms (default `10000`) for running handlers to call `ready()`, then hands unfinished interval claims back to storage (`executeAt = now`) so another instance runs them on its next poll, not after `zombieTime`. Resolves `true` if every handler finished in time. A late `ready()` from a handed-back or superseded same-process run does not touch the schedule. Cross-instance protection requires adapter-side claim-lease fencing. Repeated calls share the first shutdown attempt and timeout. At timeout, unfinished handlers are reported to `onError` (or `console.error`): current interval claims are handed back, superseded claims are left unchanged, and one-shots are abandoned to preserve at-most-once behavior. Superseded handlers still count as unfinished until they call `ready()`. Keep one-shot handlers idempotent, or set `timeout` above the longest one-shot runtime. Rejects on a negative or non-finite `timeout`. Keep `timeout` below the platform grace period (Kubernetes default 30s). ### `ping()` → `Promise` diff --git a/skills/josk/references/meteor.md b/skills/josk/references/meteor.md index 1e689f5..4f1df61 100644 --- a/skills/josk/references/meteor.md +++ b/skills/josk/references/meteor.md @@ -56,7 +56,9 @@ The scheduler shares the app's Mongo replica set — zero extra connections. ## Redis / PostgreSQL from Meteor -Same `RedisAdapter` / `PostgresAdapter` setup as `adapters.md` (Redis / KeyDB / Valkey: one writable primary; Cluster needs `useHashTags: true`). Install drivers via `meteor npm install redis` or `meteor npm install pg`, then import from `meteor/ostrio:cron-jobs` instead of `josk`. +Same `RedisAdapter` / `PostgresAdapter` setup as `adapters.md` (Redis-compatible servers: one writable primary; Cluster needs `useHashTags: true`). Meteor CI tests Redis only; KeyDB/Valkey are not tested within Meteor. Install drivers via `meteor npm install redis` or `meteor npm install pg`, then import from `meteor/ostrio:cron-jobs` instead of `josk`. + +Meteor package tests transpile TypeScript but do not type-check it. `npm run test:types` separately checks all adapter contracts and `shutdown()` through the Meteor declaration import with `tsc`. ## Meteor-specific notes diff --git a/skills/josk/references/troubleshooting.md b/skills/josk/references/troubleshooting.md index dcbbb65..55b19b5 100644 --- a/skills/josk/references/troubleshooting.md +++ b/skills/josk/references/troubleshooting.md @@ -20,7 +20,7 @@ Tuning: - `zombieTime` must exceed the slowest legitimate handler runtime plus a margin. Below 60s is not recommended. - Re-registering a claimed interval keeps its zombie deadline. An uncleanly killed handler can wait the full `zombieTime` (15 minutes by default) before recovery, rather than only `delay`; this avoids early overlap on rolling restarts. Call `await jobs.shutdown({ timeout })` on `SIGTERM` to hand running claims back. Lower `zombieTime` only if every legitimate handler finishes sooner. -- Built-in adapters fence `update()` on the claim lease: a handler that finishes after its claim was recovered (or handed back by `shutdown()`) gets `false` and leaves the newer schedule alone. +- Built-in adapters fence `update()` on the claim lease across instances. JoSk also suppresses late writes when a newer same-uid run starts in the same process, including for adapters without claim leases. Custom adapters need equivalent storage fencing to prevent an old handler on another process from overwriting the newer schedule. - Older records can carry a stale `claimLeaseId` after completion. The first new registration may preserve that date; the next `update()` clears it. Mixed-version peers can still shorten claims. - If a handler routinely hits `zombieTime`, either the handler is too slow or `zombieTime` is too tight. Split long work or move it off the scheduler. @@ -92,7 +92,7 @@ In order of likelihood: - **Reading JoSk state from a Mongo secondary or a Redis / KeyDB / Valkey replica.** Lease writes must be immediately visible. Use the primary. - **Active-active Redis / KeyDB active-replication / multi-master.** Conflict resolution can allow duplicate claims. Use a single writable primary, or `PostgresAdapter`. - **`MOVED` errors with `redis@4` Cluster on josk ≤ 6.3.** The `redis@4` cluster client routes `EVAL` to a random master (~4% failures with 3 masters); a failed `ready()` write parks the interval until `zombieTime`. 6.4 sends scripts via `sendCommand(firstKey)`. Upgrade. -- **Redis / KeyDB / Valkey Cluster without `useHashTags: true`.** Lua touches `schedule` + `tasks` + `lock`. Untagged keys hash to different slots → `CROSSSLOT`. `RedisAdapter` constructor throws for a cluster client (`nodeClient`, no `scanIterator`) without it. +- **Redis / KeyDB / Valkey Cluster without `useHashTags: true`.** Lua touches `schedule` + `tasks` + `lock`. Untagged keys hash to different slots → `CROSSSLOT`. `RedisAdapter` throws for a cluster client (`nodeClient`, no `scanIterator`) without it. CI Cluster coverage uses Redis; KeyDB/Valkey Cluster are not separately tested. - **MailTime with only one side tagged.** `RedisQueue({ useHashTags })` and JoSk `useHashTags` must match. MailTime queue layout is not a JoSk key rename — see `mail-time` skill. - **MongoDB without `w: 'majority'`.** A claim that's only on the primary can vanish on failover. Use majority writes and `readConcern: 'majority'`. - **Postgres read replicas.** Same rule — no scheduler reads on replicas. diff --git a/test/jest/core.test.js b/test/jest/core.test.js index 3802b7e..afd9356 100644 --- a/test/jest/core.test.js +++ b/test/jest/core.test.js @@ -491,6 +491,44 @@ describe('JoSk core', () => { info.mockRestore(); }); + it('supersedes earlier in-flight same-uid runs without claim leases', async () => { + const { job, adapter } = createJob(); + const uid = 'recoveredsetInterval'; + const olderTask = { uid, delay: 100, isInterval: true, isDeleted: false }; + const newerTask = { uid, delay: 200, isInterval: true, isDeleted: false }; + const readyFunctions = []; + job.tasks[uid] = (ready) => { + readyFunctions.push(ready); + }; + + await job.__execute(olderTask); + await job.__execute(newerTask); + await readyFunctions[0](); + await readyFunctions[1](); + + expect(adapter.updateCalls).toHaveLength(1); + expect(adapter.updateCalls[0].task).toBe(newerTask); + }); + + it('debugs missing claim leases once per instance', async () => { + const info = jest.spyOn(console, 'info').mockImplementation(() => {}); + const { job } = createJob({ debug: true }); + const uid = 'missing-lease-setInterval'; + const task = { uid, delay: 100, isInterval: true, isDeleted: false }; + job.tasks[uid] = () => {}; + + try { + await job.__execute({ ...task, claimLeaseId: 'lease-1' }); + await job.__execute(task); + await job.__execute(task); + + expect(info).toHaveBeenCalledTimes(1); + expect(info.mock.calls[0].join(' ')).toContain('claimLeaseId'); + } finally { + info.mockRestore(); + } + }); + it('reschedules interval with custom Date from ready()', async () => { const { job, adapter } = createJob(); const next = new Date(Date.now() + 5000); @@ -1105,8 +1143,48 @@ describe('shutdown and claim release', () => { expect(+adapter.updateCalls[0].nextExecuteAt).toBeGreaterThanOrEqual(before + task.delay); }); - it('releases unfinished interval claims after the timeout and ignores their late ready()', async () => { + it('waits for superseded handlers without overwriting the latest schedule', async () => { const { job, adapter } = createJob(); + const task = intervalTask('superseded-drain'); + const finishes = []; + job.tasks[task.uid] = (ready) => { finishes.push(ready); }; + await job.__execute(task); + await job.__execute({ ...task, claimLeaseId: 'newer' }); + await finishes[1](); + + let resolved = false; + const shutdown = job.shutdown({ timeout: 1000 }).then((result) => { + resolved = true; + return result; + }); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(resolved).toBe(false); + await finishes[0](); + await expect(shutdown).resolves.toBe(true); + expect(adapter.updateCalls).toHaveLength(1); + }); + + it('returns false for an unfinished superseded handler without releasing its stale claim', async () => { + const onError = jest.fn(); + const { job, adapter } = createJob({ onError }); + const task = intervalTask('superseded-timeout'); + const finishes = []; + job.tasks[task.uid] = (ready) => { finishes.push(ready); }; + await job.__execute(task); + await job.__execute({ ...task, claimLeaseId: 'newer' }); + await finishes[1](); + + await expect(job.shutdown({ timeout: 0 })).resolves.toBe(false); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][1].description).toContain('superseded'); + expect(onError.mock.calls[0][1].description).not.toContain('handed back'); + await finishes[0](); + expect(adapter.updateCalls).toHaveLength(1); + }); + + it('releases unfinished interval claims after the timeout and ignores their late ready()', async () => { + const onError = jest.fn(); + const { job, adapter } = createJob({ onError }); let finish; const task = intervalTask('stuck'); job.tasks[task.uid] = (ready) => { @@ -1116,6 +1194,10 @@ describe('shutdown and claim release', () => { await job.__execute(task); await expect(job.shutdown({ timeout: 10 })).resolves.toBe(false); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][1].uid).toBe(task.uid); + expect(onError.mock.calls[0][1].description).toContain('interval'); + expect(onError.mock.calls[0][1].description).toContain('handed back'); expect(adapter.updateCalls).toHaveLength(1); expect(adapter.updateCalls[0].task).toBe(task); expect(+adapter.updateCalls[0].nextExecuteAt).toBeLessThanOrEqual(Date.now()); @@ -1124,15 +1206,130 @@ describe('shutdown and claim release', () => { expect(adapter.updateCalls).toHaveLength(1); }); - it('does not release one-shot tasks, which storage already removed', async () => { - const { job, adapter } = createJob(); + it('does not report one-shot work that finishes within timeout', async () => { + const onError = jest.fn(); + const { job, adapter } = createJob({ onError }); + let finish; + const uid = 'finishes-before-timeoutsetTimeout'; + job.tasks[uid] = (ready) => { + finish = ready; + }; + adapter.stored.set(uid, { uid }); + + await job.__execute({ uid, delay: 0, isInterval: false, isDeleted: false }); + const shutdown = job.shutdown({ timeout: 100 }); + setTimeout(() => finish(), 10); + + await expect(shutdown).resolves.toBe(true); + expect(onError).not.toHaveBeenCalled(); + expect(adapter.updateCalls).toHaveLength(0); + expect(adapter.addCalls).toHaveLength(0); + }); + + it('reports and abandons unfinished one-shot tasks without re-adding them', async () => { + const onError = jest.fn(); + const { job, adapter } = createJob({ onError }); + let finish; const uid = 'stuck-oncesetTimeout'; - job.tasks[uid] = (_ready) => {}; + job.tasks[uid] = (ready) => { + finish = ready; + }; adapter.stored.set(uid, { uid }); await job.__execute({ uid, delay: 0, isInterval: false, isDeleted: false }); await expect(job.shutdown({ timeout: 10 })).resolves.toBe(false); + + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][0]).toBe('[shutdown] timeout'); + expect(onError.mock.calls[0][1].uid).toBe(uid); + expect(onError.mock.calls[0][1].description).toContain('one-shot'); + expect(onError.mock.calls[0][1].description).toContain('abandoned'); + expect(onError.mock.calls[0][1].error).toBeInstanceOf(Error); + expect(adapter.removeCalls).toEqual([uid]); + expect(adapter.updateCalls).toHaveLength(0); + expect(adapter.addCalls).toHaveLength(0); + + await expect(finish()).resolves.toBe(true); + expect(adapter.updateCalls).toHaveLength(0); + expect(adapter.addCalls).toHaveLength(0); + }); + + it('reports unfinished interval and one-shot outcomes separately', async () => { + const onError = jest.fn(); + const { job, adapter } = createJob({ onError }); + const interval = intervalTask('mixed-interval'); + const oneShotUid = 'mixed-oneshotsetImmediate'; + let finishInterval; + let finishOneShot; + job.tasks[interval.uid] = (ready) => { + finishInterval = ready; + }; + job.tasks[oneShotUid] = (ready) => { + finishOneShot = ready; + }; + adapter.stored.set(oneShotUid, { uid: oneShotUid }); + + await job.__execute(interval); + await job.__execute({ uid: oneShotUid, delay: 0, isInterval: false, isDeleted: false }); + await expect(job.shutdown({ timeout: 10 })).resolves.toBe(false); + + expect(onError).toHaveBeenCalledTimes(2); + const reports = onError.mock.calls.map(([, details]) => details); + expect(reports.map(({ uid }) => uid)).toEqual([interval.uid, oneShotUid]); + expect(reports[0].description).toContain('interval'); + expect(reports[0].description).toContain('handed back'); + expect(reports[1].description).toContain('one-shot'); + expect(reports[1].description).toContain('abandoned'); + expect(adapter.updateCalls).toHaveLength(1); + expect(adapter.updateCalls[0].task).toBe(interval); + expect(+adapter.updateCalls[0].nextExecuteAt).toBeLessThanOrEqual(Date.now()); + expect(adapter.removeCalls).toEqual([oneShotUid]); + expect(adapter.addCalls).toHaveLength(0); + + await Promise.all([finishInterval(), finishOneShot()]); + expect(adapter.updateCalls).toHaveLength(1); + }); + + it('shares timeout outcome and report when shutdown is called twice', async () => { + const onError = jest.fn(); + const { job, adapter } = createJob({ onError }); + let finish; + const uid = 'twice-oncesetTimeout'; + job.tasks[uid] = (ready) => { + finish = ready; + }; + adapter.stored.set(uid, { uid }); + + await job.__execute({ uid, delay: 0, isInterval: false, isDeleted: false }); + const firstShutdown = job.shutdown({ timeout: 10 }); + const secondShutdown = job.shutdown({ timeout: 100 }); + + await expect(firstShutdown).resolves.toBe(false); + await expect(secondShutdown).resolves.toBe(false); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][1].uid).toBe(uid); expect(adapter.updateCalls).toHaveLength(0); + expect(adapter.addCalls).toHaveLength(0); + + await expect(finish()).resolves.toBe(true); + }); + + it('reports an unfinished one-shot when timeout is zero', async () => { + const onError = jest.fn(); + const { job, adapter } = createJob({ onError }); + const uid = 'zero-timeoutsetImmediate'; + job.tasks[uid] = (_ready) => {}; + adapter.stored.set(uid, { uid }); + + await job.__execute({ uid, delay: 0, isInterval: false, isDeleted: false }); + await expect(job.shutdown({ timeout: 0 })).resolves.toBe(false); + + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][1].uid).toBe(uid); + expect(onError.mock.calls[0][1].description).toContain('one-shot'); + expect(onError.mock.calls[0][1].description).toContain('abandoned'); + expect(adapter.updateCalls).toHaveLength(0); + expect(adapter.addCalls).toHaveLength(0); }); it('releases a task claimed after destroy() without running its handler', async () => { diff --git a/test/meteor-types.ts b/test/meteor-types.ts index 974fe11..cbac0e1 100644 --- a/test/meteor-types.ts +++ b/test/meteor-types.ts @@ -1,6 +1,6 @@ import { MongoInternals } from 'meteor/mongo'; import { assert } from 'chai'; -import { JoSk, MongoAdapter } from 'meteor/ostrio:cron-jobs'; +import { JoSk, MongoAdapter, PostgresAdapter, RedisAdapter } from 'meteor/ostrio:cron-jobs'; import type { JoSkAdapter, JoSkExecuteMode, @@ -27,8 +27,14 @@ after(function () { describe('Meteor TypeScript — types compile-time checks', () => { it('Built-in adapters satisfy JoSkAdapter contract', () => { type _MongoAdapterIsJoSkAdapter = MongoAdapter extends JoSkAdapter ? true : never; + type _RedisAdapterIsJoSkAdapter = RedisAdapter extends JoSkAdapter ? true : never; + type _PostgresAdapterIsJoSkAdapter = PostgresAdapter extends JoSkAdapter ? true : never; const _mongoOk: _MongoAdapterIsJoSkAdapter = true; + const _redisOk: _RedisAdapterIsJoSkAdapter = true; + const _postgresOk: _PostgresAdapterIsJoSkAdapter = true; assert.equal(_mongoOk, true, 'MongoAdapter assignable to JoSkAdapter'); + assert.equal(_redisOk, true, 'RedisAdapter assignable to JoSkAdapter'); + assert.equal(_postgresOk, true, 'PostgresAdapter assignable to JoSkAdapter'); }); it('Hook aliases accept sync and async signatures', () => { @@ -141,4 +147,14 @@ describe('Meteor TypeScript — runtime', function () { assert.equal(typedJobs.resume(intervalId), true, 'resume(timerId) returns boolean'); assert.equal(typedJobs.resume(), true, 'resume() returns boolean'); }); + + it('shutdown returns Promise', async function () { + if (!typedJobs) { + assert.fail('typedJobs not initialized'); + return; + } + + const shutdownResult: Promise = typedJobs.shutdown({ timeout: 1000 }); + assert.isTrue(await shutdownResult, 'shutdown resolves true when no handlers are running'); + }); }); diff --git a/test/mongo-url.js b/test/mongo-url.js new file mode 100644 index 0000000..96a061a --- /dev/null +++ b/test/mongo-url.js @@ -0,0 +1,16 @@ +export const getMongoDatabaseName = (mongoUrl) => { + const schemeEnd = mongoUrl.indexOf('://'); + if (schemeEnd === -1) { + return ''; + } + + const pathStart = mongoUrl.indexOf('/', schemeEnd + 3); + if (pathStart === -1) { + return ''; + } + + const queryStart = mongoUrl.indexOf('?', pathStart); + const pathEnd = queryStart === -1 ? mongoUrl.length : queryStart; + const pathname = mongoUrl.slice(pathStart + 1, pathEnd).replace(/\/$/, ''); + return decodeURIComponent(pathname); +}; diff --git a/test/mongo-url.test.js b/test/mongo-url.test.js new file mode 100644 index 0000000..7ebb08b --- /dev/null +++ b/test/mongo-url.test.js @@ -0,0 +1,17 @@ +import { assert } from 'chai'; +import { describe, it } from 'mocha'; +import { getMongoDatabaseName } from './mongo-url.js'; + +describe('Mongo connection URL parsing', () => { + it('keeps query options out of the database name', () => { + const url = 'mongodb://user:pass@db.example.test:27017/josk_test?tls=true&retryWrites=false'; + + assert.equal(getMongoDatabaseName(url), 'josk_test'); + }); + + it('parses the database name with comma-separated replica-set hosts', () => { + const url = 'mongodb://host1.example.test:27017,host2.example.test:27017/josk_test?tls=true&replicaSet=rs0'; + + assert.equal(getMongoDatabaseName(url), 'josk_test'); + }); +}); diff --git a/test/npm-mongo.js b/test/npm-mongo.js index 26198f7..57caa0f 100644 --- a/test/npm-mongo.js +++ b/test/npm-mongo.js @@ -5,6 +5,7 @@ import { CronExpressionParser } from 'cron-parser'; import { it, describe, before, after } from 'mocha'; import { assert } from 'chai'; import { closeMongoClient, destroyJobs, uniqueId, wait, waitUntil } from './helpers.js'; +import { getMongoDatabaseName } from './mongo-url.js'; import { registerIntervalRestartTests } from './interval-restart-tests.js'; import { registerPauseResumeTests } from './pause-resume-tests.js'; @@ -32,7 +33,7 @@ const racingJoSkOpts = { }; const mongoAddr = (process.env.MONGO_URL || ''); -const dbName = mongoAddr.split('/').pop().replace(/\/$/, ''); +const dbName = getMongoDatabaseName(mongoAddr); const callbacks = {}; const exceptions = {}; diff --git a/test/npm.js b/test/npm.js index bcceedd..d01695f 100644 --- a/test/npm.js +++ b/test/npm.js @@ -1,3 +1,4 @@ import './npm-mongo.js'; +import './mongo-url.test.js'; import './npm-redis.js'; import './npm-postgres.js'; diff --git a/test/types/packed.mjs b/test/types/packed.mjs index db0f847..74462d5 100644 --- a/test/types/packed.mjs +++ b/test/types/packed.mjs @@ -90,9 +90,20 @@ void drained; mkdirSync(resolve(destination, '..'), { recursive: true }); copyFileSync(asset, destination); } - writeFileSync(join(project, 'meteor.ts'), `import { JoSk, MongoAdapter } from 'meteor/ostrio:cron-jobs'; + writeFileSync(join(project, 'meteor.ts'), `import { JoSk, MongoAdapter, RedisAdapter, PostgresAdapter } from 'meteor/ostrio:cron-jobs'; +import type { JoSkAdapter, JoSkShutdownOption } from 'meteor/ostrio:cron-jobs'; +type AssertAdapter = T; +type MongoContract = AssertAdapter; +type RedisContract = AssertAdapter; +type PostgresContract = AssertAdapter; const jobs = new JoSk({ adapter: new MongoAdapter({ db: { collection: () => ({}), command: async () => ({ ok: 1 }) } }) }); -jobs.destroy(); +const options: JoSkShutdownOption = { timeout: 1000 }; +const shutdownResult: Promise = jobs.shutdown(options); +void shutdownResult; +// @ts-expect-error The shutdown timeout must be numeric. +jobs.shutdown({ timeout: '1000' }); +// @ts-expect-error Shutdown is asynchronous, not a synchronous boolean. +const synchronousResult: boolean = jobs.shutdown(); `); writeFileSync(join(project, 'tsconfig.meteor.json'), JSON.stringify({ compilerOptions: { @@ -111,7 +122,7 @@ jobs.destroy(); cwd: project, stdio: 'inherit' }); - console.log('Meteor declaration assets resolve through the package import.'); + console.log('Meteor declaration assets type-check all adapter contracts and shutdown through the package import.'); } finally { rmSync(temp, { recursive: true, force: true }); }