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

Tolerant edge cases in LosslessJsonParser #212

Closed
ryanelee opened this issue Jul 17, 2024 · 1 comment
Closed

Tolerant edge cases in LosslessJsonParser #212

ryanelee opened this issue Jul 17, 2024 · 1 comment

Comments

@ryanelee
Copy link

When there is a edge case in the input variable payload such as array of null elements, the recursive method convertLosslessNumbersToNumberOrThrow underneath the createWorker throws cannot read properties of null error.

SDK Component

Zeebe gRPC client

Expected Behavior

The null should be ignored and the JSON string should be serialized successfully

Current Behavior

The convertLosslessNumbersToNumberOrThrow method in LosslessJsonParser.ts throws out exception on the 1st line
debug(Parsing LosslessNumbers to numbers for ${obj.constructor.name}) when the obj is null.

Error:
Cannot read properties of null (reading 'constructor')

Possible Solution

Add the null check in the convertLosslessNumbersToNumberOrThrow method in LosslessJsonParser.ts.

Add safe navigation operation on line 223:
obj?.constructor?.name

Add a null check before 224:
if (!obj) return obj;

Steps to Reproduce

  1. Added a new unit test in LosslessJsonParser.unit.spec.ts
test('LosslessJsonParser correctly handles null objects', () => {
    const json = `{"abc": [null, null, null] }`;

    const parsedDto = losslessParse(json);

    expect(parsedDto.siuTaskComments).toMatchObject([null, null, null]); // 3 (string)
});
  1. Run the unit test
  2. The new added unit test will be failed with below error:
An unsafe number value was received for "abc" and no Dto mapping was specified.
    Cannot read properties of null (reading 'constructor')

      248 |     } catch (e) {
      249 |             const message = (e as Error).message
    > 250 |             throw new Error(
          |                   ^
      251 |                     `An unsafe number value was received for "${currentKey}" and no Dto mapping was specified.\n` +
      252 |                             message
      253 |             )

Context (Environment)

The error will stop Zeebe gRPC client from polling activate tasks.

@jwulf
Copy link
Member

jwulf commented Jul 29, 2024

Thanks for finding this and suggesting a fix. I've implemented it.

@jwulf jwulf closed this as completed in b712651 Jul 29, 2024
github-actions bot pushed a commit that referenced this issue Jul 29, 2024
## [8.6.10](v8.6.9...v8.6.10) (2024-07-29)

### Bug Fixes

* **lossless-parser:** correctly handle null in objects ([b712651](b712651)), closes [#212](#212)
github-actions bot pushed a commit that referenced this issue Jul 29, 2024
## [8.6.10](v8.6.9...v8.6.10) (2024-07-29)

### Bug Fixes

* **lossless-parser:** correctly handle null in objects ([b712651](b712651)), closes [#212](#212)
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

2 participants