Standardise the Engine JSON-RPC error codes

[Malaydewangan09]

PR Description

This pull request standardises the handling of JSON-RPC error codes in the Web3JClient::doRequest method according to the JSON-RPC 2.0 specification. It introduces specific error code handling for well-known JSON-RPC errors and customizes error messages to reduce unnecessary stack trace logging.

Fixed Issue(s)

fixes #8671

Documentation

• I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

• I thought about adding a changelog entry, and added one if I deemed necessary.

[tbenr]

Thanks! Looks nice so far.
I have some comments and I think we should add some tests in Web3JClientTest

[Malaydewangan09]

Hey @tbenr
I've refactored the JsonRpcErrorCodes class to use an enum for improved structure and efficiency.
However, I'm looking for help in writing or updating the corresponding test cases for this change.

[tbenr]

Hey @Malaydewangan09 I don't see the update on the Enum front. Can you double check?
Thanks!

[Malaydewangan09]

Hey @tbenr, my bad.
I've updated the class to an Enum.
Can you please check the changes?

[tbenr]

WRT testing you could:

take inspiration from shouldNotUpdateAvailabilityWhenNonCriticalMethodFailsWithErrorResponse on how to simulate an error and from shouldTimeoutIfResponseNotReceived on how to get the response and check that the mapping has been applied. I'd add one test covering the fact that the errors are decoded via JsonRpcErrorCodes as expected.

[tbenr]

I think there is still something to improve:

maybe the final error should be something like JSON-RPC error: Invalid params (-32602) ?

[Malaydewangan09]

Ah yes, that would be a more concise error message.
I'll update the message.

[tbenr]

sorry I got confused by repetition of Invalid params. The first is the decoded error description, the second is the message coming from the server. I'd change the message to be something different in the test.

[Malaydewangan09]

@tbenr
I'd like to clarify the purpose of your comment about the repetition of 'Invalid params'. Were you suggesting we make the test case more realistic by using a distinct server error message, or was there another reason for pointing out this repetition?

[tbenr]

@tbenr I'd like to clarify the purpose of your comment about the repetition of 'Invalid params'. Were you suggesting we make the test case more realistic by using a distinct server error message, or was there another reason for pointing out this repetition?

@Malaydewangan09 Yes, I was suggesting to make the test more realistic and thus less confusing. The message from the server could be something more verbose like "engine_newPaylad method has been called with invalid parameters"

[tbenr]

LGTM. Thanks for the contribution!

[Malaydewangan09]

@tbenr Thanks for your consistent feedback!