Skip to content
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.

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

Zeebe Worker payload parsing needs to be lossless #81

Closed
jwulf opened this issue Mar 21, 2024 · 5 comments · Fixed by #95
Closed

Zeebe Worker payload parsing needs to be lossless #81

jwulf opened this issue Mar 21, 2024 · 5 comments · Fixed by #95
Assignees
Milestone

Comments

@jwulf
Copy link
Member

jwulf commented Mar 21, 2024

Similar to #80, the Zeebe worker variable payload may contain int64 number values.

Without a user-provided Dto class to leverage #78, we can't know the variable type - but we can detect an unsafe number value if we parse with lossless-json.

An option could be to specify via configuration how variables with a number value that is unsafe are handled.

If there is no Dto provided, we could default to throwing during the parse operation. This stops the application from operating with bad data.

And we could allow the user to set a default behaviour, "coerce to BigInt" or "coerce to string".

@jwulf jwulf added this to the 8.5.0 milestone Mar 21, 2024
@jwulf jwulf self-assigned this Mar 21, 2024
@jwulf
Copy link
Member Author

jwulf commented Mar 22, 2024

The Zeebe worker payload comes over as stringified JSON, so int64 values will be numbers to the parser.

@jwulf
Copy link
Member Author

jwulf commented Mar 26, 2024

As the first step towards this, the Zeebe worker input and output (the job.variables received by the handler function, and the input to the complete function) are now parsed and stringified using the lossless-number library.

This means that int64 number values outside the safe range of a JavaScript number sent over the wire from the Zeebe broker will cause the SDK to throw an exception when it parses the stringified JSON.

This is lossless, but not predictable for consumers.

The next phase is to allow the user to specify a LosslessDto class as a decoder schema.

@jwulf
Copy link
Member Author

jwulf commented Mar 27, 2024

OK, the LosslessDto for the worker variable decoding is implemented.

Typing of Zeebe Worker Variables

The variable payload in a Zeebe Worker task handler is available as an object job.variables. By default, this is of type any.

The ZBClient.createWorker() method accepts an inputVariableDto to control the parsing of number values and provide design-time type information. Passing an inputVariableDto class to a Zeebe worker is optional. If a Dto class is passed to the Zeebe Worker, it is used for two purposes:

  1. To provide design-time type information on the job.variables object.
  2. To specify the parsing of JSON number fields. These can potentially represent int64 values that cannot be represented accurately by the JavaScript number type. With a Dto, you can specify that a specific JSON number fields be parsed losslessly to a string or BigInt.

With no Dto specified, there is no design-time type safety. At run-time, all JSON numbers are converted to the JavaScript number type. If a variable field has a number value that cannot be safely represented using the JavaScript number type (a value greater than 2^53 -1) then an exception is thrown.

To provide a Dto, extend the LosslessDto class, like so:

class MyVariableDto extends LosslessDto {
  name!: string
  maybeAge?: number
  @Int64String
  veryBigNumber!: string
  @BigIntValue
   veryBigInteger!: bigint
}

In this case, veryBigNumber is an int64 value. It is transferred as a JSON number on the wire, but the parser will parse it into a string so that no loss of precision occurs. Similarly, veryBigInteger is a very large integer value. In this case, we direct the parser to parse this variable field as a bigint.

You can nest Dtos like this:

class MyLargerDto extends LosslessDto {
  id!: string
  @ChildDto(MyVariableDto)
  entry!: MyVariableDto
}

Typing of Custom Headers

The Zeebe worker receives custom headers as job.customHeaders. The ZBClient.createWorker() method accepts a customHeadersDto to control the behaviour of custom header parsing of number values and provide design-time type information.

This follows the same strategy as the job variables, as previously described.

@jwulf
Copy link
Member Author

jwulf commented Mar 27, 2024

OK, let's make this simple.

These are the scenarios that we want to handle:

  1. We do not want to have JavaScript numbers in the application that claim to represent a value but are incorrect.
  2. We want to allow the consumer to specify how these values should be represented in the application, as bigint or string.
  3. We want this to be consistent - in other words, we don't want a variable field that has a different type depending on its value.
  4. We don't want to force the consumer to make all numbers bigint or string, only the ones that could carry an unsafe int64 value.

To accomplish these:

  1. The parser should throw if it encounters an unsafe int64 value that is not mapped to either string or bigint.
  2. We should accept a Dto that can declare that number fields are mapped to either string or bigint.
  3. Any number that is not mapped and is safe to represent as number will be parsed to number. This means it is always an accurate number, or the application throws.
  4. See 3.

So there is one type of parser, one type of Dto.

The parser, if not supplied with a Dto, will simply parse the variables using lossless-json, then iterate over the resulting object, converting all LosslessNumbers to number or throwing if they are unsafe. This requires no type information or mapping.

This will also be the last stage for the parser when supplied with a Dto. When it is supplied with a Dto, the parser will iterate over the Dto, and convert the LosslessNumber of the parsed object to the mapped type of the Dto.

A question arises here: How should the parser behave if it encounters a value that is not a LosslessNumber when a mapping has been specified?

Answer: it should throw an error, because the expectation expressed in the Dto does not meet reality, and as a consequence the application behaviour is not predictable from the code.

Final stage: the object should be iterated with the parser to convert any remaining (unmapped) LosslessNumbers to number type or throw.

A question arises here: a field may be specified in the Dto as number but in fact it has an int64 value in it. If this is detected by the presence of an unsafe number, the parser should definitely throw.

But what about when the Dto is not complete? After the application is deployed, the payload is extended and now contains an int64 field that is not in the Dto. The application gets all the variables, and an unsafe, unmapped value shows up. How should the parser behave? If it throws, it forces the application developer to either constrain the variables requested to those in the Dto, or to extend the Dto.

Answer: it should throw if it there is an unmapped, unsafe value.

@jwulf
Copy link
Member Author

jwulf commented Mar 27, 2024

Next step:

Deal with Output Variable Dto and stringifying variables.

@jwulf jwulf closed this as completed in #95 Mar 28, 2024
jwulf added a commit that referenced this issue Mar 28, 2024
* feat(zeebe): implement lossless parsing of job payload

FIxes #81

* feat(zeebe): implement decisive parser strategy

* feat(repo): implement lossless stringify
github-actions bot pushed a commit that referenced this issue Apr 4, 2024
# [8.5.0-alpha.1](v8.4.0...v8.5.0-alpha.1) (2024-04-04)

### Features

* **oauth:** support optional scope in OAuth request ([#25](#25)) ([0451b80](0451b80))
* **operate:** add multitenant support and lossless Json parsing ([#82](#82)) ([cf49a71](cf49a71)), closes [#78](#78) [#67](#67)
* **repo:** add enhanced debug output for http errors ([#88](#88)) ([881b039](881b039)), closes [#87](#87)
* **tasklist:** add multitenant support to tasklist ([#85](#85)) ([46bb564](46bb564))
* **zeebe:** add MigrateProcessInstance ([#97](#97)) ([2a9a123](2a9a123)), closes [#49](#49)
* **zeebe:** implement deleteResource ([#73](#73)) ([0cd08b7](0cd08b7))
* **zeebe:** implement lossless parsing of job payload ([#95](#95)) ([57f3ea8](57f3ea8)), closes [#81](#81)
* **zeebe:** normalise useragent, thread config ([#94](#94)) ([c1c4211](c1c4211))
* **zeebe:** remove deployProcess method ([#71](#71)) ([6cb98f0](6cb98f0))
jwulf added a commit that referenced this issue Apr 8, 2024
* ci(repo): create npm publish workflow (#100)

* Remove-docs (#101)

* docs(repo): remove docs

* docs(repo): add docs folder to .gitignore

* ci(repo): update publish workflow (#102)

* ci(repo): configure renovate to run against alpha branch (#103)

* ci(repo): configure renovate to run against alpha branch

* ci(repo): add workflow step to merge to alpha from main on release

* chore(release): 8.5.0-alpha.1 [skip ci]

# [8.5.0-alpha.1](v8.4.0...v8.5.0-alpha.1) (2024-04-04)

### Features

* **oauth:** support optional scope in OAuth request ([#25](#25)) ([0451b80](0451b80))
* **operate:** add multitenant support and lossless Json parsing ([#82](#82)) ([cf49a71](cf49a71)), closes [#78](#78) [#67](#67)
* **repo:** add enhanced debug output for http errors ([#88](#88)) ([881b039](881b039)), closes [#87](#87)
* **tasklist:** add multitenant support to tasklist ([#85](#85)) ([46bb564](46bb564))
* **zeebe:** add MigrateProcessInstance ([#97](#97)) ([2a9a123](2a9a123)), closes [#49](#49)
* **zeebe:** implement deleteResource ([#73](#73)) ([0cd08b7](0cd08b7))
* **zeebe:** implement lossless parsing of job payload ([#95](#95)) ([57f3ea8](57f3ea8)), closes [#81](#81)
* **zeebe:** normalise useragent, thread config ([#94](#94)) ([c1c4211](c1c4211))
* **zeebe:** remove deployProcess method ([#71](#71)) ([6cb98f0](6cb98f0))

* test(repo): skip jest global setup for unit tests (#106)


Fixes #105

* build(repo): parallelise publish workflow

* build(repo): fix parallelisation

* fix(repo): add note on "supported" (#107)

Fixes #70

* chore(release): 8.5.0-alpha.2 [skip ci]

# [8.5.0-alpha.2](v8.5.0-alpha.1...v8.5.0-alpha.2) (2024-04-05)

### Bug Fixes

* **repo:** add note on "supported" ([#107](#107)) ([fc45d61](fc45d61)), closes [#70](#70)

* fix(repo): only git commit on npm publish success

* chore(release): 8.5.0-alpha.3 [skip ci]

# [8.5.0-alpha.3](v8.5.0-alpha.2...v8.5.0-alpha.3) (2024-04-05)

### Bug Fixes

* **repo:** only git commit on npm publish success ([9012764](9012764))

* chore(release): 8.5.0-alpha.3 [skip ci]

# [8.5.0-alpha.3](v8.5.0-alpha.2...v8.5.0-alpha.3) (2024-04-05)

### Bug Fixes

* **repo:** only git commit on npm publish success ([9012764](9012764))

* build(repo): configure semantic release (#109)

fixes #108

* fix(repo): use ts-patch to transform module mapping in output (#112)

* fix(repo): use ts-patch to transform module mapping in output

fixes #110

* build(repo): add missing tsconfig-paths

* chore(release): 8.5.0-alpha.4 [skip ci]

# [8.5.0-alpha.4](v8.5.0-alpha.3...v8.5.0-alpha.4) (2024-04-05)

### Bug Fixes

* **repo:** use ts-patch to transform module mapping in output ([#112](#112)) ([7efdcf3](7efdcf3)), closes [#110](#110)

* chore(release): 8.5.0-alpha.4 [skip ci]

# [8.5.0-alpha.4](v8.5.0-alpha.3...v8.5.0-alpha.4) (2024-04-05)

### Bug Fixes

* **repo:** use ts-patch to transform module mapping in output ([#112](#112)) ([7efdcf3](7efdcf3)), closes [#110](#110)

* ci(repo): parallelise tests and use npm install in CI (#116)

fixes #115

* Jwulf/issue114 (#117)

* fix(repo): use relative paths in source

fixes #114

* test(repo): fix relative path in jest setup

* test(repo): fix relative path to zeebe

* test(repo): convert tests to relative paths

* revert(github): remove spurious commitlint config file

* fix(repo): make fix type commits release a new package

* ci(repo): wrap long lines in commit messages

* ci(repo): wrap footer lines at 100 characters in semantic-release config

* ci(repo): break commit msgs to 100 chars via husky hook

* ci(repo): disable husky during semantic-release

* chore(release): 8.5.0-alpha.5 [skip ci]

# [8.5.0-alpha.5](v8.5.0-alpha.4...v8.5.0-alpha.5) (2024-04-07)

### Bug Fixes

* **repo:** make fix type commits release a new package ([ded83cf](ded83cf))

* chore(release): 8.5.0-alpha.5 [skip ci]

# [8.5.0-alpha.5](v8.5.0-alpha.4...v8.5.0-alpha.5) (2024-04-07)

### Bug Fixes

* **repo:** make fix type commits release a new package ([ded83cf](ded83cf))

* Issue118 (#119)

* test(repo): add smoke test and type surface tests
fixes #118
* ci(repo): run smoke test in CI
* fix(camunda8): respect OAuth disabled flag

* fix(issue118): add smoke test and type surface tests

* test(repo): add smoke test and type surface tests
fixes #118
* ci(repo): run smoke test in CI
* fix(camunda8): respect OAuth disabled flag

* release(repo): trigger a new release

* chore(release): 8.5.0-alpha.6 [skip ci]

# [8.5.0-alpha.6](v8.5.0-alpha.5...v8.5.0-alpha.6) (2024-04-08)

### Bug Fixes

* **issue118:** add smoke test and type surface tests ([fe0c709](fe0c709)), closes [#118](#118)

* chore(release): 8.5.0-alpha.6 [skip ci]

# [8.5.0-alpha.6](v8.5.0-alpha.5...v8.5.0-alpha.6) (2024-04-08)

### Bug Fixes

* **issue118:** add smoke test and type surface tests ([fe0c709](fe0c709)), closes [#118](#118)

* ci(repo): add CodeQL scanning (#120)

fixes #51

---------

Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>
github-actions bot pushed a commit that referenced this issue Apr 8, 2024
## [8.4.1](v8.4.0...v8.4.1) (2024-04-08)

### Features

* **oauth:** support optional scope in OAuth request ([#25](#25)) ([0451b80](0451b80))
* **operate:** add multitenant support and lossless Json parsing ([#82](#82)) ([cf49a71](cf49a71)), closes [#78](#78) [#67](#67)
* **repo:** add enhanced debug output for http errors ([#88](#88)) ([881b039](881b039)), closes [#87](#87)
* **tasklist:** add multitenant support to tasklist ([#85](#85)) ([46bb564](46bb564))
* **zeebe:** add MigrateProcessInstance ([#97](#97)) ([2a9a123](2a9a123)), closes [#49](#49)
* **zeebe:** implement deleteResource ([#73](#73)) ([0cd08b7](0cd08b7))
* **zeebe:** implement lossless parsing of job payload ([#95](#95)) ([57f3ea8](57f3ea8)), closes [#81](#81)
* **zeebe:** normalise useragent, thread config ([#94](#94)) ([c1c4211](c1c4211))
* **zeebe:** remove deployProcess method ([#71](#71)) ([6cb98f0](6cb98f0))
github-actions bot pushed a commit that referenced this issue Apr 8, 2024
## [8.4.1](v8.4.0...v8.4.1) (2024-04-08)

### Features

* **oauth:** support optional scope in OAuth request ([#25](#25)) ([0451b80](0451b80))
* **operate:** add multitenant support and lossless Json parsing ([#82](#82)) ([cf49a71](cf49a71)), closes [#78](#78) [#67](#67)
* **repo:** add enhanced debug output for http errors ([#88](#88)) ([881b039](881b039)), closes [#87](#87)
* **tasklist:** add multitenant support to tasklist ([#85](#85)) ([46bb564](46bb564))
* **zeebe:** add MigrateProcessInstance ([#97](#97)) ([2a9a123](2a9a123)), closes [#49](#49)
* **zeebe:** implement deleteResource ([#73](#73)) ([0cd08b7](0cd08b7))
* **zeebe:** implement lossless parsing of job payload ([#95](#95)) ([57f3ea8](57f3ea8)), closes [#81](#81)
* **zeebe:** normalise useragent, thread config ([#94](#94)) ([c1c4211](c1c4211))
* **zeebe:** remove deployProcess method ([#71](#71)) ([6cb98f0](6cb98f0))
jwulf added a commit that referenced this issue Apr 8, 2024
* Release 8.5.0 (#124)

* ci(repo): create npm publish workflow (#100)

* Remove-docs (#101)

* docs(repo): remove docs

* docs(repo): add docs folder to .gitignore

* ci(repo): update publish workflow (#102)

* ci(repo): configure renovate to run against alpha branch (#103)

* ci(repo): configure renovate to run against alpha branch

* ci(repo): add workflow step to merge to alpha from main on release

* chore(release): 8.5.0-alpha.1 [skip ci]

# [8.5.0-alpha.1](v8.4.0...v8.5.0-alpha.1) (2024-04-04)

### Features

* **oauth:** support optional scope in OAuth request ([#25](#25)) ([0451b80](0451b80))
* **operate:** add multitenant support and lossless Json parsing ([#82](#82)) ([cf49a71](cf49a71)), closes [#78](#78) [#67](#67)
* **repo:** add enhanced debug output for http errors ([#88](#88)) ([881b039](881b039)), closes [#87](#87)
* **tasklist:** add multitenant support to tasklist ([#85](#85)) ([46bb564](46bb564))
* **zeebe:** add MigrateProcessInstance ([#97](#97)) ([2a9a123](2a9a123)), closes [#49](#49)
* **zeebe:** implement deleteResource ([#73](#73)) ([0cd08b7](0cd08b7))
* **zeebe:** implement lossless parsing of job payload ([#95](#95)) ([57f3ea8](57f3ea8)), closes [#81](#81)
* **zeebe:** normalise useragent, thread config ([#94](#94)) ([c1c4211](c1c4211))
* **zeebe:** remove deployProcess method ([#71](#71)) ([6cb98f0](6cb98f0))

* test(repo): skip jest global setup for unit tests (#106)


Fixes #105

* build(repo): parallelise publish workflow

* build(repo): fix parallelisation

* fix(repo): add note on "supported" (#107)

Fixes #70

* chore(release): 8.5.0-alpha.2 [skip ci]

# [8.5.0-alpha.2](v8.5.0-alpha.1...v8.5.0-alpha.2) (2024-04-05)

### Bug Fixes

* **repo:** add note on "supported" ([#107](#107)) ([fc45d61](fc45d61)), closes [#70](#70)

* fix(repo): only git commit on npm publish success

* chore(release): 8.5.0-alpha.3 [skip ci]

# [8.5.0-alpha.3](v8.5.0-alpha.2...v8.5.0-alpha.3) (2024-04-05)

### Bug Fixes

* **repo:** only git commit on npm publish success ([9012764](9012764))

* chore(release): 8.5.0-alpha.3 [skip ci]

# [8.5.0-alpha.3](v8.5.0-alpha.2...v8.5.0-alpha.3) (2024-04-05)

### Bug Fixes

* **repo:** only git commit on npm publish success ([9012764](9012764))

* build(repo): configure semantic release (#109)

fixes #108

* fix(repo): use ts-patch to transform module mapping in output (#112)

* fix(repo): use ts-patch to transform module mapping in output

fixes #110

* build(repo): add missing tsconfig-paths

* chore(release): 8.5.0-alpha.4 [skip ci]

# [8.5.0-alpha.4](v8.5.0-alpha.3...v8.5.0-alpha.4) (2024-04-05)

### Bug Fixes

* **repo:** use ts-patch to transform module mapping in output ([#112](#112)) ([7efdcf3](7efdcf3)), closes [#110](#110)

* chore(release): 8.5.0-alpha.4 [skip ci]

# [8.5.0-alpha.4](v8.5.0-alpha.3...v8.5.0-alpha.4) (2024-04-05)

### Bug Fixes

* **repo:** use ts-patch to transform module mapping in output ([#112](#112)) ([7efdcf3](7efdcf3)), closes [#110](#110)

* ci(repo): parallelise tests and use npm install in CI (#116)

fixes #115

* Jwulf/issue114 (#117)

* fix(repo): use relative paths in source

fixes #114

* test(repo): fix relative path in jest setup

* test(repo): fix relative path to zeebe

* test(repo): convert tests to relative paths

* revert(github): remove spurious commitlint config file

* fix(repo): make fix type commits release a new package

* ci(repo): wrap long lines in commit messages

* ci(repo): wrap footer lines at 100 characters in semantic-release config

* ci(repo): break commit msgs to 100 chars via husky hook

* ci(repo): disable husky during semantic-release

* chore(release): 8.5.0-alpha.5 [skip ci]

# [8.5.0-alpha.5](v8.5.0-alpha.4...v8.5.0-alpha.5) (2024-04-07)

### Bug Fixes

* **repo:** make fix type commits release a new package ([ded83cf](ded83cf))

* chore(release): 8.5.0-alpha.5 [skip ci]

# [8.5.0-alpha.5](v8.5.0-alpha.4...v8.5.0-alpha.5) (2024-04-07)

### Bug Fixes

* **repo:** make fix type commits release a new package ([ded83cf](ded83cf))

* Issue118 (#119)

* test(repo): add smoke test and type surface tests
fixes #118
* ci(repo): run smoke test in CI
* fix(camunda8): respect OAuth disabled flag

* fix(issue118): add smoke test and type surface tests

* test(repo): add smoke test and type surface tests
fixes #118
* ci(repo): run smoke test in CI
* fix(camunda8): respect OAuth disabled flag

* release(repo): trigger a new release

* chore(release): 8.5.0-alpha.6 [skip ci]

# [8.5.0-alpha.6](v8.5.0-alpha.5...v8.5.0-alpha.6) (2024-04-08)

### Bug Fixes

* **issue118:** add smoke test and type surface tests ([fe0c709](fe0c709)), closes [#118](#118)

* chore(release): 8.5.0-alpha.6 [skip ci]

# [8.5.0-alpha.6](v8.5.0-alpha.5...v8.5.0-alpha.6) (2024-04-08)

### Bug Fixes

* **issue118:** add smoke test and type surface tests ([fe0c709](fe0c709)), closes [#118](#118)

* ci(repo): add CodeQL scanning (#120)

fixes #51

---------

Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>

* release: trigger a new release

* chore(release): 8.4.1 [skip ci]

## [8.4.1](v8.4.0...v8.4.1) (2024-04-08)

### Features

* **oauth:** support optional scope in OAuth request ([#25](#25)) ([0451b80](0451b80))
* **operate:** add multitenant support and lossless Json parsing ([#82](#82)) ([cf49a71](cf49a71)), closes [#78](#78) [#67](#67)
* **repo:** add enhanced debug output for http errors ([#88](#88)) ([881b039](881b039)), closes [#87](#87)
* **tasklist:** add multitenant support to tasklist ([#85](#85)) ([46bb564](46bb564))
* **zeebe:** add MigrateProcessInstance ([#97](#97)) ([2a9a123](2a9a123)), closes [#49](#49)
* **zeebe:** implement deleteResource ([#73](#73)) ([0cd08b7](0cd08b7))
* **zeebe:** implement lossless parsing of job payload ([#95](#95)) ([57f3ea8](57f3ea8)), closes [#81](#81)
* **zeebe:** normalise useragent, thread config ([#94](#94)) ([c1c4211](c1c4211))
* **zeebe:** remove deployProcess method ([#71](#71)) ([6cb98f0](6cb98f0))

* chore(release): 8.4.1 [skip ci]

## [8.4.1](v8.4.0...v8.4.1) (2024-04-08)

### Features

* **oauth:** support optional scope in OAuth request ([#25](#25)) ([0451b80](0451b80))
* **operate:** add multitenant support and lossless Json parsing ([#82](#82)) ([cf49a71](cf49a71)), closes [#78](#78) [#67](#67)
* **repo:** add enhanced debug output for http errors ([#88](#88)) ([881b039](881b039)), closes [#87](#87)
* **tasklist:** add multitenant support to tasklist ([#85](#85)) ([46bb564](46bb564))
* **zeebe:** add MigrateProcessInstance ([#97](#97)) ([2a9a123](2a9a123)), closes [#49](#49)
* **zeebe:** implement deleteResource ([#73](#73)) ([0cd08b7](0cd08b7))
* **zeebe:** implement lossless parsing of job payload ([#95](#95)) ([57f3ea8](57f3ea8)), closes [#81](#81)
* **zeebe:** normalise useragent, thread config ([#94](#94)) ([c1c4211](c1c4211))
* **zeebe:** remove deployProcess method ([#71](#71)) ([6cb98f0](6cb98f0))

---------

Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

1 participant