Skip to content

Making Router handle JSON RPC Errors #117

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.

Sign up for GitHub

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

Merged
willmeister merged 2 commits into from
Apr 29, 2020
Merged

Making Router handle JSON RPC Errors #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
300c84c
Making it so the router does not throw on properly-formatted JSON err…
willmeister Apr 29, 2020
2994097
Fixing errors in Routing Handler tests and adding tests for JSON erro…
willmeister Apr 29, 2020
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
5 changes: 4 additions & 1 deletion packages/core-utils/src/app/test-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,24 @@ export class TestUtils {
public static async assertThrowsAsync(
func: () => Promise<any>,
errorType?: any
): Promise<void> {
): Promise<Error> {
let succeeded = true
let error: Error
try {
await func()
succeeded = false
} catch (e) {
if (!!errorType && !(e instanceof errorType)) {
succeeded = false
}
error = e
}

assert(
succeeded,
"Function didn't throw as expected or threw the wrong error."
)
return error
}

public static async assertRevertsAsync(
Expand Down
33 changes: 26 additions & 7 deletions packages/core-utils/src/app/transport/client/json-rpc-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
JsonRpcRequest,
Client,
isJsonRpcErrorResponse,
JsonRpcResponse,
} from '../../../types'

/**
Expand All @@ -23,11 +24,34 @@ export class JsonRpcClient<TransportRequest, TransportResponse>
/**
* Handles a method call by making a JSON-RPC
* request to some server.
*
* @param method Name of the method to call.
* @param [params] Parameters to send with the method call.
* @returns the result of the method call.
* @returns the `result` field of the response to the method call.
* @throws Error if there is any error, including a properly-formatted JsonRpcResponse error.
*/
public async handle<T>(method: string, params?: any): Promise<T> {
const response: JsonRpcResponse = await this.makeRpcCall(method, params)

if (isJsonRpcErrorResponse(response)) {
throw new Error(`${JSON.stringify(response.error)}`)
}
return response.result
}

/**
* Makes an RPC call and returns the full JsonRpcResponse.
* Notably, this differs from handle<T>(...) because it does not throw on error
* or just return the `result` field on success.
*
* @param method Name of the method to call.
* @param [params] Parameters to send with the method call.
* @returns the result of the method call.
*/
public async makeRpcCall(
method: string,
params?: any
): Promise<JsonRpcResponse> {
const request: JsonRpcRequest = {
jsonrpc: '2.0',
method,
Expand All @@ -37,11 +61,6 @@ export class JsonRpcClient<TransportRequest, TransportResponse>

const encodedRequest = this.adapter.encodeRequest(request)
const encodedResponse = await this.client.request(encodedRequest)
const response = this.adapter.decodeResponse(encodedResponse)

if (isJsonRpcErrorResponse(response)) {
throw new Error(`${JSON.stringify(response.error)}`)
}
return response.result
return this.adapter.decodeResponse(encodedResponse)
}
}
20 changes: 18 additions & 2 deletions packages/core-utils/src/app/transport/client/simple-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Wrapper class around a Http-based JsonRpcClient
*/
import { JsonRpcClient } from './json-rpc-client'
import { HttpRequest, HttpResponse } from '../../../types'
import { HttpRequest, HttpResponse, JsonRpcResponse } from '../../../types'
import { JsonRpcHttpAdapter } from './json-rpc-http-adapter'
import { AxiosHttpClient } from './axios-http-client'

Expand All @@ -25,9 +25,25 @@ export class SimpleClient {
* request to some server.
* @param method Name of the method to call.
* @param [params] Parameters to send with the method call.
* @returns the result of the method call.
* @returns the `result` field of the response to the method call.
* @throws Error if there is any error, including a properly-formatted JsonRpcResponse error.
*/
public async handle<T>(method: string, params?: any): Promise<T> {
return this.jsonRpcClient.handle<T>(method, params)
}

/**
* Makes an RPC call and returns the full JsonRpcResponse.
* Notably, this differs from handle<T>(...) because it does not throw one error
* or just return the `result` field on success.
* @param method Name of the method to call.
* @param [params] Parameters to send with the method call.
* @returns the result of the method call.
*/
public async makeRpcCall(
method: string,
params?: any
): Promise<JsonRpcResponse> {
return this.jsonRpcClient.makeRpcCall(method, params)
}
}
9 changes: 9 additions & 0 deletions packages/rollup-full-node/src/app/fullnode-rpc-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {

/* Internal Imports */
import {
FormattedJsonRpcError,
FullnodeHandler,
InvalidParametersError,
InvalidTransactionDesinationError,
Expand Down Expand Up @@ -102,6 +103,14 @@ export class FullnodeRpcServer extends ExpressHttpServer {
result,
}
} catch (err) {
if (err instanceof FormattedJsonRpcError) {
log.debug(
`Received formatted JSON RPC Error response. Returning it as is: ${JSON.stringify(
err.jsonRpcResponse
)}`
)
return err.jsonRpcResponse
}
if (err instanceof RevertError) {
log.debug(`Request reverted. Request: ${JSON.stringify(request)}`)
const errorResponse: JsonRpcErrorResponse = buildJsonRpcError(
Expand Down
21 changes: 16 additions & 5 deletions packages/rollup-full-node/src/app/routing-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
/* External Imports */
import { Address } from '@eth-optimism/rollup-core'
import { getLogger, logError, SimpleClient } from '@eth-optimism/core-utils'
import {
getLogger,
isJsonRpcErrorResponse,
JsonRpcErrorResponse,
JsonRpcResponse,
logError,
SimpleClient,
} from '@eth-optimism/core-utils'

import { parseTransaction, Transaction } from 'ethers/utils'

/* Internal Imports */
import {
AccountRateLimiter,
FormattedJsonRpcError,
FullnodeHandler,
InvalidParametersError,
InvalidTransactionDesinationError,
Expand Down Expand Up @@ -95,16 +103,19 @@ export class RoutingHandler implements FullnodeHandler {
this.assertDestinationValid(tx)

try {
const result: any =
const result: JsonRpcResponse =
methodsToRouteWithTransactionHandler.indexOf(method) >= 0
? await this.transactionClient.handle<string>(method, params)
: await this.readOnlyClient.handle<string>(method, params)
? await this.transactionClient.makeRpcCall(method, params)
: await this.readOnlyClient.makeRpcCall(method, params)
log.debug(
`Request for [${method}], params: [${JSON.stringify(
params
)}] got result [${JSON.stringify(result)}]`
)
return result
if (isJsonRpcErrorResponse(result)) {
throw new FormattedJsonRpcError(result as JsonRpcErrorResponse)
}
return result.result
} catch (e) {
logError(
log,
Expand Down
8 changes: 8 additions & 0 deletions packages/rollup-full-node/src/types/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { JsonRpcErrorResponse } from '@eth-optimism/core-utils'

export class TreeUpdateError extends Error {
constructor(message?: string) {
super(message || 'Error occurred performing a tree update!')
Expand Down Expand Up @@ -62,3 +64,9 @@ export class InvalidTransactionDesinationError extends Error {
)
}
}

export class FormattedJsonRpcError extends Error {
constructor(public readonly jsonRpcResponse: JsonRpcErrorResponse) {
super()
}
}
131 changes: 108 additions & 23 deletions packages/rollup-full-node/test/app/routing-handler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
/* External Imports */
import { SimpleClient, TestUtils } from '@eth-optimism/core-utils'

import {
JsonRpcError,
JsonRpcErrorResponse,
JsonRpcResponse,
JsonRpcSuccessResponse,
SimpleClient,
TestUtils,
} from '@eth-optimism/core-utils'
/* Internal Imports */
import {
AccountRateLimiter,
allWeb3RpcMethodsIncludingTest,
FormattedJsonRpcError,
InvalidTransactionDesinationError,
RateLimitError,
TransactionLimitError,
Expand All @@ -19,11 +26,15 @@ import {
} from '../../src/app'

class DummySimpleClient extends SimpleClient {
constructor(private readonly cannedResponse: any) {
constructor(private readonly cannedResponse: JsonRpcResponse) {
super('')
}
public async handle<T>(method: string, params?: any): Promise<T> {
return this.cannedResponse as T

public async makeRpcCall(
method: string,
params?: any
): Promise<JsonRpcResponse> {
return this.cannedResponse
}
}

Expand Down Expand Up @@ -56,13 +67,24 @@ const getSignedTransaction = async (
})
}

const transactionResponse = 'transaction'
const readOnlyResponse = 'read only'
const transactionResponsePayload: JsonRpcSuccessResponse = {
jsonrpc: '2.0',
id: 123,
result: transactionResponse,
}
const readOnlyPayload: JsonRpcSuccessResponse = {
jsonrpc: '2.0',
id: 1234,
result: readOnlyResponse,
}

describe('Routing Handler', () => {
describe('Routing Tests', () => {
const transactionResponse = 'transaction'
const readOnlyResponse = 'read only'
const routingHandler = new RoutingHandler(
new DummySimpleClient(transactionResponse),
new DummySimpleClient(readOnlyResponse),
new DummySimpleClient(transactionResponsePayload),
new DummySimpleClient(readOnlyPayload),
'',
new NoOpAccountRateLimiter(),
[],
Expand Down Expand Up @@ -97,14 +119,11 @@ describe('Routing Handler', () => {
let rateLimiter: DummyRateLimiter
let routingHandler: RoutingHandler

const transactionResponse = 'transaction'
const readOnlyResponse = 'read only'

beforeEach(() => {
rateLimiter = new DummyRateLimiter()
routingHandler = new RoutingHandler(
new DummySimpleClient(transactionResponse),
new DummySimpleClient(readOnlyResponse),
new DummySimpleClient(transactionResponsePayload),
new DummySimpleClient(readOnlyPayload),
'',
rateLimiter,
[],
Expand Down Expand Up @@ -156,15 +175,13 @@ describe('Routing Handler', () => {
describe('unsupported destination tests', () => {
let routingHandler: RoutingHandler

const transactionResponse = 'transaction'
const readOnlyResponse = 'read only'
const deployerWallet: Wallet = Wallet.createRandom()
const whitelistedTo: string = Wallet.createRandom().address

beforeEach(() => {
routingHandler = new RoutingHandler(
new DummySimpleClient(transactionResponse),
new DummySimpleClient(readOnlyResponse),
new DummySimpleClient(transactionResponsePayload),
new DummySimpleClient(readOnlyPayload),
deployerWallet.address,
new NoOpAccountRateLimiter(),
[whitelistedTo],
Expand Down Expand Up @@ -218,13 +235,10 @@ describe('Routing Handler', () => {
describe('unsupported methods tests', () => {
let routingHandler: RoutingHandler

const transactionResponse = 'transaction'
const readOnlyResponse = 'read only'

beforeEach(() => {
routingHandler = new RoutingHandler(
new DummySimpleClient(transactionResponse),
new DummySimpleClient(readOnlyResponse),
new DummySimpleClient(transactionResponsePayload),
new DummySimpleClient(readOnlyPayload),
'',
new NoOpAccountRateLimiter(),
[],
Expand All @@ -251,4 +265,75 @@ describe('Routing Handler', () => {
}, UnsupportedMethodError)
})
})

describe('Formatted JSON RPC Responses', () => {
let routingHandler: RoutingHandler

const txError: JsonRpcError = {
code: -123,
message: 'tx error',
data: 'tx error',
}

const roError: JsonRpcError = {
code: -1234,
message: 'r/o error',
data: 'r/o error',
}

const transactionErrorResponsePayload: JsonRpcErrorResponse = {
jsonrpc: '2.0',
id: 123,
error: txError,
}
const readOnlyErrorResponsePayload: JsonRpcErrorResponse = {
jsonrpc: '2.0',
id: 1234,
error: roError,
}

beforeEach(() => {
routingHandler = new RoutingHandler(
new DummySimpleClient(transactionErrorResponsePayload),
new DummySimpleClient(readOnlyErrorResponsePayload),
'',
new NoOpAccountRateLimiter(),
[]
)
})

it('throws Json error on transaction', async () => {
const error: Error = await TestUtils.assertThrowsAsync(async () => {
await routingHandler.handleRequest(
Web3RpcMethods.sendRawTransaction,
[await getSignedTransaction()],
''
)
})

error.should.be.instanceOf(FormattedJsonRpcError, 'Invalid error type!')
const formatted: FormattedJsonRpcError = error as FormattedJsonRpcError
formatted.jsonRpcResponse.should.deep.equal(
transactionErrorResponsePayload,
'Incorrect error returned!'
)
})

it('throws Json error on read only request', async () => {
const error: Error = await TestUtils.assertThrowsAsync(async () => {
await routingHandler.handleRequest(
Web3RpcMethods.networkVersion,
[],
''
)
})

error.should.be.instanceOf(FormattedJsonRpcError, 'Invalid error type!')
const formatted: FormattedJsonRpcError = error as FormattedJsonRpcError
formatted.jsonRpcResponse.should.deep.equal(
readOnlyErrorResponsePayload,
'Incorrect error returned!'
)
})
})
})