Skip to content

fix: declare listOffsets isolationLevel as a number in the options schema - #433

Merged
ShogunPanda merged 1 commit into
platformatic:mainfrom
fadiroot:fix/list-offsets-isolation-level-schema
Oct 1, 2026
Merged

ShogunPanda merged 1 commit into
platformatic:mainfrom
fadiroot:fix/list-offsets-isolation-level-schema

Conversation

@fadiroot

Copy link
Copy Markdown
Contributor

Summary

src/clients/consumer/options.ts declares the listOffsets option isolationLevel as

isolationLevel: { type: 'string', enum: allowedFetchIsolationLevels },

but allowedFetchIsolationLevels is Object.values(FetchIsolationLevels), i.e. the numbers 0 and 1. With strict: true (Ajv runs with coerceTypes: false) no value can satisfy the schema: FetchIsolationLevels.READ_COMMITTED (1) fails the type check with /options/isolationLevel must be string, and '1' fails the enum. listOffsetsWithTimestamps goes through the same schema.

The consumer's own fetch options (isolationLevel: { type: 'number', enum: allowedFetchIsolationLevels }) and the admin schema already use the number type; this PR aligns the listOffsets schema with them.

Test

listOffsets should accept a custom isolation level in strict mode in test/clients/consumer/consumer.test.ts, a strict-mode twin of the existing non-strict test. On main it fails with /options/isolationLevel must be string.; with this change it passes against a Kafka 3.9.1 broker. pnpm lint is clean.

…hema

The listOffsets option schema declared isolationLevel as a string with
the FetchIsolationLevels enum, but those values are the numbers 0 and 1.
With `strict: true` no value could pass validation: `1` failed the type
check and `'1'` failed the enum, so strict consumers could not call
listOffsets or listOffsetsWithTimestamps with an isolation level at all.

Use the number type, as the consumer's own fetch options and the admin
schema already do.

Signed-off-by: Fadi Romdhan <fadiromdhan3@gmail.com>
@ShogunPanda
ShogunPanda merged commit 66d867a into platformatic:main Oct 1, 2026
58 of 59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants