Skip to content

ensure null value are parsed correctly#108

Merged
EnriqueL8 merged 1 commit into
hyperledger-firefly:mainfrom
kaleido-io:fixing-null-value-parsing
Jul 7, 2026
Merged

ensure null value are parsed correctly#108
EnriqueL8 merged 1 commit into
hyperledger-firefly:mainfrom
kaleido-io:fixing-null-value-parsing

Conversation

@Chengxuan

Copy link
Copy Markdown
Contributor

As part of https://github.com/hyperledger-firefly/signer/pull/100/changes#diff-2aad17c4690b0c1fa07321da007ac247e464434b5619e1350b48b8a0a63550d6L169 I switched the CallRPC function to use the internal function syncRequestTyped for metrics recording based on http response status.

However, this introduced a regression because the safeguarding logic of responses that contain result: null is in the wrapper function SyncRequest. This PR fixes the regression by applying the safeguarding logic in the regressed code path with a test added.

Also, refactored out the common logic.

Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
@Chengxuan
Chengxuan requested a review from a team as a code owner July 6, 2026 13:15

@EnriqueL8 EnriqueL8 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @Chengxuan

Comment thread pkg/rpcbackend/backend.go
// treating an absent payload as JSON null. A success response with "result": null
// decodes to a nil JSONAny
func unmarshalRPCResult(data *fftypes.JSONAny, target interface{}) error {
b := data.Bytes()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually doesn't this nil pointer exception? don't you need to check data != nil ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not needed. Covered by the underlying Bytes() function

@EnriqueL8
EnriqueL8 merged commit d1600f0 into hyperledger-firefly:main Jul 7, 2026
3 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.

2 participants