Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Handling of the "null" response from provider #5324

Merged
merged 2 commits into from
Aug 8, 2022
Merged

Handling of the "null" response from provider #5324

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
571354b
:bug: Fix the null response handling
nazarhussain Aug 7, 2022
ea8e1d9
Merge branch '4.x' into nh/5319-handling-null-response
nazarhussain Aug 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 43 additions & 23 deletions packages/web3-core/src/web3_request_manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,16 @@ export class Web3RequestManager<
response: JsonRpcResponse<ResultType, ErrorType>,
{ legacy, error }: { legacy: boolean; error: boolean },
): JsonRpcResponse<ResultType> | never {
if (isNullish(response)) {
Copy link
Contributor

@jdevcs jdevcs Aug 8, 2022

Choose a reason for hiding this comment

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

can you share rpc request and eth node you used for this case?

Copy link
Contributor Author

@nazarhussain nazarhussain Aug 8, 2022

Choose a reason for hiding this comment

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

It was tested for eth_getTransactionReceipt with ganache for a transaction hash which is not part of the block yet.

https://github.com/ethereum/execution-apis/blob/main/src/eth/transaction.yaml#L42

return this._buildResponse(
payload,
// Some providers uses "null" as valid empty response
// eslint-disable-next-line no-null/no-null
null as unknown as JsonRpcResponse<ResultType, ErrorType>,
error,
);
}

// This is the majority of the cases so check these first
// A valid JSON-RPC response with error object
if (jsonRpc.isResponseWithError<ErrorType>(response)) {
Expand Down Expand Up @@ -308,29 +318,7 @@ export class Web3RequestManager<
!jsonRpc.isResponseWithError(response) &&
!jsonRpc.isResponseWithResult(response)
) {
const res = {
jsonrpc: '2.0',
// eslint-disable-next-line no-nested-ternary
id: jsonRpc.isBatchRequest(payload)
? payload[0].id
: 'id' in payload
? payload.id
: // Have to use the null here explicitly
// eslint-disable-next-line no-null/no-null
null,
};

if (error) {
return {
...res,
error: response as unknown,
} as JsonRpcResponse<ResultType>;
}

return {
...res,
result: response as unknown,
} as JsonRpcResponse<ResultType>;
return this._buildResponse(payload, response, error);
}

if (jsonRpc.isBatchRequest(payload) && !Array.isArray(response)) {
Expand All @@ -352,4 +340,36 @@ export class Web3RequestManager<

throw new ResponseError(response, 'Invalid response');
}

// Need to use same types as _processJsonRpcResponse so have to declare as instance method
// eslint-disable-next-line class-methods-use-this
private _buildResponse<ResultType, ErrorType, RequestType>(
payload: JsonRpcPayload<RequestType>,
response: JsonRpcResponse<ResultType, ErrorType>,
error: boolean,
): JsonRpcResponse<ResultType> {
const res = {
jsonrpc: '2.0',
// eslint-disable-next-line no-nested-ternary
id: jsonRpc.isBatchRequest(payload)
? payload[0].id
: 'id' in payload
? payload.id
: // Have to use the null here explicitly
// eslint-disable-next-line no-null/no-null
null,
};

if (error) {
return {
...res,
error: response as unknown,
} as JsonRpcResponse<ResultType>;
}

return {
...res,
result: response as unknown,
} as JsonRpcResponse<ResultType>;
}
}
71 changes: 38 additions & 33 deletions packages/web3-core/test/unit/web3_request_manager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,9 @@ describe('Web3RequestManager', () => {

describe('web3-provider', () => {
beforeEach(() => {
// isWeb3Provider uses instanceof to check if the provider is a Web3Provider
// So we have to mock the response
jest.spyOn(utils, 'isWeb3Provider').mockReturnValue(true);
jest.spyOn(utils, 'isLegacyRequestProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendAsyncProvider').mockReturnValue(false);
});

it('should pass request to provider and resolve if provider resolves it', async () => {
Expand Down Expand Up @@ -318,14 +317,6 @@ describe('Web3RequestManager', () => {
});

describe('legacy-request-provider', () => {
beforeEach(() => {
jest.spyOn(utils, 'isWeb3Provider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacyRequestProvider').mockReturnValue(true);
jest.spyOn(utils, 'isLegacySendProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendAsyncProvider').mockReturnValue(false);
jest.spyOn(utils, 'isEIP1193Provider').mockReturnValue(false);
});

it('should pass request to provider and resolve if provider resolves it', async () => {
const manager = new Web3RequestManager();
const myProvider = {
Expand Down Expand Up @@ -382,10 +373,8 @@ describe('Web3RequestManager', () => {

describe('eip1193-provider', () => {
beforeEach(() => {
jest.spyOn(utils, 'isWeb3Provider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacyRequestProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendAsyncProvider').mockReturnValue(false);
// isEIP1193Provider uses typeof to check if the provider is a EIP1193Provider
// So we have to mock the response
jest.spyOn(utils, 'isEIP1193Provider').mockReturnValue(true);
});

Expand Down Expand Up @@ -439,10 +428,8 @@ describe('Web3RequestManager', () => {

describe('eip1193-provider - return non json-rpc compliance response', () => {
beforeEach(() => {
jest.spyOn(utils, 'isWeb3Provider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacyRequestProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendAsyncProvider').mockReturnValue(false);
// isEIP1193Provider uses typeof to check if the provider is a EIP1193Provider
// So we have to mock the response
jest.spyOn(utils, 'isEIP1193Provider').mockReturnValue(true);
});

Expand Down Expand Up @@ -475,16 +462,41 @@ describe('Web3RequestManager', () => {
expect(myProvider.request).toHaveBeenCalledTimes(1);
expect(myProvider.request).toHaveBeenCalledWith(payload);
});
});

describe('legacy-send-provider', () => {
beforeEach(() => {
jest.spyOn(utils, 'isWeb3Provider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacyRequestProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendProvider').mockReturnValue(true);
jest.spyOn(utils, 'isLegacySendAsyncProvider').mockReturnValue(false);
it('should pass request to provider and pass if provider returns "null', async () => {
const manager = new Web3RequestManager();
const myProvider = {
request: jest.fn().mockImplementation(async _ => {
// Explicitly used for test case
// eslint-disable-next-line no-null/no-null
return null;
}),
} as any;

jest.spyOn(manager, 'provider', 'get').mockReturnValue(myProvider);

await expect(manager.send(request)).resolves.toBeNull();
expect(myProvider.request).toHaveBeenCalledTimes(1);
expect(myProvider.request).toHaveBeenCalledWith(payload);
});

it('should pass request to provider and pass if provider returns "undefined', async () => {
const manager = new Web3RequestManager();
const myProvider = {
request: jest.fn().mockImplementation(async _ => {
return undefined;
}),
} as any;

jest.spyOn(manager, 'provider', 'get').mockReturnValue(myProvider);

await expect(manager.send(request)).resolves.toBeNull();
expect(myProvider.request).toHaveBeenCalledTimes(1);
expect(myProvider.request).toHaveBeenCalledWith(payload);
});
});

describe('legacy-send-provider', () => {
it('should pass request to provider and resolve if provider resolves it', async () => {
const manager = new Web3RequestManager();
const myProvider = {
Expand Down Expand Up @@ -540,13 +552,6 @@ describe('Web3RequestManager', () => {
});

describe('legacy-send-async-provider', () => {
beforeEach(() => {
jest.spyOn(utils, 'isWeb3Provider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacyRequestProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendProvider').mockReturnValue(false);
jest.spyOn(utils, 'isLegacySendAsyncProvider').mockReturnValue(true);
});

it('should pass request to provider and resolve if provider resolves it', async () => {
const manager = new Web3RequestManager();
const myProvider = {
Expand Down