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

Lossless stringify and parse do not handle Date, Map, or Set #254

Closed
jwulf opened this issue Sep 19, 2024 · 1 comment
Closed

Lossless stringify and parse do not handle Date, Map, or Set #254

jwulf opened this issue Sep 19, 2024 · 1 comment
Assignees
Milestone

Comments

@jwulf
Copy link
Member

jwulf commented Sep 19, 2024

Any variables that are managed in application code as type Date, Map, or Set will not be correctly serialised.

Claude 3.5 recommends the following patch for Date:

if (value instanceof Date) {
  // Handle Date objects by converting them to an ISO string
  newObj[key] = value.toISOString();
}

And suggests flatted, or json-stringify-safe for Map and Set.

I checked those out, and they are for circular JSON. We are not going to support that - we've going to throw on circular JSON. And they don't have any support for Map or Set.

An issue here is that even with the patch that Claude suggests, we are not going to get a reviver without metadata. So if users are dealing with a Date object, then send it to the Camunda 8 engine, when it next shows up (at the end of a process or in a worker) it will be of type string.

So: we should throw when passed a Date, Map, or Set, and make the de/serialization to string the concern of the application.

And as a bonus, I could make a Dto annotation that de/serializes those types.

So when encountering a Date, Map, or Set, the stringifier checks if it is a LosslessDto and has the correct metadata field. If it does, we assume that the user is leaning into the Dto framework to handle this concern, and do a transform.

Let me do a spike on this and see how practical it is. I'd like to support native types.

The risk area is that a user sends data with a Dto with the correct metadata, but elsewhere receives it with a different Dto and it deserialises as a different type. We can throw if we are stringifying something with insufficient metadata, but we can't know when parsing it.

@jwulf
Copy link
Member Author

jwulf commented Sep 19, 2024

I went lowest-orbit possible.

Because I can't reliable rehydrate across the system boundary, I'm going to make it explicitly the concern of the application.

We throw (with a helpful message) if we get a Date, Map, or Set in variables.

@jwulf jwulf self-assigned this Sep 19, 2024
@jwulf jwulf added this to the 8.6.0 milestone Sep 19, 2024
@jwulf jwulf closed this as completed in bb5d8ea Oct 24, 2024
github-actions bot pushed a commit that referenced this issue Oct 24, 2024
## [8.6.14](v8.6.13...v8.6.14) (2024-10-24)

### Bug Fixes

* **camunda8:** correctly parse autostart parameter of JobWorker ([cb95946](cb95946))
* **camunda8:** type variables in async process instance start as never ([3055734](3055734))
* **lossless-parser:** correctly parse number array ([d69729a](d69729a)), closes [#258](#258)
* **lossless-parser:** throw on encountering Date, Map, or Set ([bb5d8ea](bb5d8ea)), closes [#254](#254)
* **zeebe:** do not override explicit ZEEBE_GRPC_ADDRESS with default ZEEBE_ADDRESS ([cd6080f](cd6080f)), closes [#245](#245)
* **zeebe:** throw on client if array passed as variables to CompleteJob ([40a6316](40a6316)), closes [#247](#247)

### Features

* **camunda8:** add C8RestClient ([8e93c92](8e93c92)), closes [#235](#235)
* **camunda8:** add modifyAuthorization method ([0d97f68](0d97f68))
* **camunda8:** complete deployResources feature ([8043ac9](8043ac9))
* **camunda8:** implement createProcessInstanceWithResult ([4ec4fa1](4ec4fa1))
* **camunda8:** implement deleteResource over REST ([1dcb101](1dcb101)), closes [#251](#251)
* **camunda8:** implement deployResources REST API ([debd212](debd212))
* **camunda8:** implement publishMessage over REST ([057a9fe](057a9fe)), closes [#250](#250)
* **camunda8:** support broadcastSignal over REST ([43f82a4](43f82a4)), closes [#248](#248)
* **camunda8:** support pluggable winston logging for C8RestClient ([d41d3f8](d41d3f8))
* **camunda8:** support updateElementInstanceVariables ([7de82b7](7de82b7)), closes [#249](#249)
* **repo:** support passing middleware ([1b7715e](1b7715e)), closes [#261](#261)
* **zeebe:** add operationReference field to gRPC methods ([2e5af66](2e5af66)), closes [#237](#237)
* **zeebe:** create and cancel process instances over REST ([a49d217](a49d217))
* **zeebe:** lossless parse REST variables and customheaders ([f19a252](f19a252)), closes [#244](#244)
github-actions bot pushed a commit that referenced this issue Oct 24, 2024
## [8.6.14](v8.6.13...v8.6.14) (2024-10-24)

### Bug Fixes

* **camunda8:** correctly parse autostart parameter of JobWorker ([cb95946](cb95946))
* **camunda8:** type variables in async process instance start as never ([3055734](3055734))
* **lossless-parser:** correctly parse number array ([d69729a](d69729a)), closes [#258](#258)
* **lossless-parser:** throw on encountering Date, Map, or Set ([bb5d8ea](bb5d8ea)), closes [#254](#254)
* **zeebe:** do not override explicit ZEEBE_GRPC_ADDRESS with default ZEEBE_ADDRESS ([cd6080f](cd6080f)), closes [#245](#245)
* **zeebe:** throw on client if array passed as variables to CompleteJob ([40a6316](40a6316)), closes [#247](#247)

### Features

* **camunda8:** add C8RestClient ([8e93c92](8e93c92)), closes [#235](#235)
* **camunda8:** add modifyAuthorization method ([0d97f68](0d97f68))
* **camunda8:** complete deployResources feature ([8043ac9](8043ac9))
* **camunda8:** implement createProcessInstanceWithResult ([4ec4fa1](4ec4fa1))
* **camunda8:** implement deleteResource over REST ([1dcb101](1dcb101)), closes [#251](#251)
* **camunda8:** implement deployResources REST API ([debd212](debd212))
* **camunda8:** implement publishMessage over REST ([057a9fe](057a9fe)), closes [#250](#250)
* **camunda8:** support broadcastSignal over REST ([43f82a4](43f82a4)), closes [#248](#248)
* **camunda8:** support pluggable winston logging for C8RestClient ([d41d3f8](d41d3f8))
* **camunda8:** support updateElementInstanceVariables ([7de82b7](7de82b7)), closes [#249](#249)
* **repo:** support passing middleware ([1b7715e](1b7715e)), closes [#261](#261)
* **zeebe:** add operationReference field to gRPC methods ([2e5af66](2e5af66)), closes [#237](#237)
* **zeebe:** create and cancel process instances over REST ([a49d217](a49d217))
* **zeebe:** lossless parse REST variables and customheaders ([f19a252](f19a252)), closes [#244](#244)
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

No branches or pull requests

1 participant