-
Notifications
You must be signed in to change notification settings - Fork 759
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
Client: fix rpc debug #3125
Client: fix rpc debug #3125
Changes from all commits
3a13477
5cc67e3
e3b2fc3
16f0466
3a4f4c6
7088542
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
type CreateRPCServerOpts = { | ||
methodConfig: MethodConfig | ||
rpcDebug: string | ||
rpcDebugVerbose: string | ||
logger?: Logger | ||
} | ||
type CreateRPCServerReturn = { | ||
|
@@ -42,6 +43,28 @@ | |
/** Allowed drift for jwt token issuance is 60 seconds */ | ||
const ALLOWED_DRIFT = 60_000 | ||
|
||
/** | ||
* Check if the `method` matches the comma-separated filter string | ||
* @param method - Method to check the filter on | ||
* @param filterStringCSV - Comma-separated list of filters to use | ||
* @returns | ||
*/ | ||
function checkFilter(method: string, filterStringCSV: string) { | ||
if (filterStringCSV === '') { | ||
return false | ||
} | ||
if (filterStringCSV === 'all') { | ||
return true | ||
} | ||
const filters = filterStringCSV.split(',') | ||
for (const filter of filters) { | ||
if (method.includes(filter) === true) { | ||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
/** | ||
* Internal util to pretty print params for logging. | ||
*/ | ||
|
@@ -63,51 +86,23 @@ | |
manager: RPCManager, | ||
opts: CreateRPCServerOpts | ||
): CreateRPCServerReturn { | ||
const { methodConfig, rpcDebug, logger } = opts | ||
|
||
const { methodConfig, rpcDebug, rpcDebugVerbose, logger } = opts | ||
const onRequest = (request: any) => { | ||
let msg = '' | ||
if (rpcDebug && rpcDebug !== '') { | ||
let show = false | ||
if (rpcDebug === 'all') { | ||
show = true | ||
} else { | ||
const filters = rpcDebug.split(',') | ||
for (const filter of filters) { | ||
if (request.method.includes(filter) === true) { | ||
show = true | ||
} | ||
} | ||
} | ||
if (show) { | ||
if (logger?.level === 'debug') { | ||
msg += `${request.method} called with params:\n${inspectParams(request.params)}` | ||
} else { | ||
msg += `${request.method} called with params: ${inspectParams(request.params, 125)}` | ||
} | ||
logger?.info(msg) | ||
} | ||
if (checkFilter(request.method, rpcDebugVerbose)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To note, the options are:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, so for extra for clarification: the |
||
logger?.info(`${request.method} called with params:\n${inspectParams(request.params)}`) | ||
} else if (checkFilter(request.method, rpcDebug)) { | ||
logger?.info(`${request.method} called with params: ${inspectParams(request.params, 125)}`) | ||
} | ||
} | ||
|
||
const handleResponse = (request: any, response: any, batchAddOn = '') => { | ||
let msg = '' | ||
if ( | ||
rpcDebug === 'true' || | ||
(rpcDebug === 'engine' && request.method.includes('engine_') === true) || | ||
(rpcDebug === 'eth' && request.method.includes('eth_') === true) | ||
) { | ||
msg = `${request.method}${batchAddOn} responded with:\n${inspectParams(response)}` | ||
} else { | ||
msg = `${request.method}${batchAddOn} responded with: ` | ||
if (response.result !== undefined) { | ||
msg += inspectParams(response, 125) | ||
} | ||
if (response.error !== undefined) { | ||
msg += `error: ${response.error.message}` | ||
} | ||
if (checkFilter(request.method, rpcDebugVerbose)) { | ||
logger?.info(`${request.method}${batchAddOn} responded with:\n${inspectParams(response)}`) | ||
} else if (checkFilter(request.method, rpcDebug)) { | ||
logger?.info( | ||
`${request.method}${batchAddOn} responded with:\n${inspectParams(response, 125)}` | ||
) | ||
} | ||
logger?.debug(msg) | ||
} | ||
|
||
const onBatchResponse = (request: any, response: any) => { | ||
|
@@ -126,11 +121,14 @@ | |
} | ||
|
||
let methods | ||
const ethMethods = manager.getMethods(false, rpcDebug !== 'false') | ||
const ethMethods = manager.getMethods(false, rpcDebug !== 'false' && rpcDebug !== '') | ||
|
||
switch (methodConfig) { | ||
case MethodConfig.WithEngine: | ||
methods = { ...ethMethods, ...manager.getMethods(true, rpcDebug !== 'false') } | ||
methods = { | ||
...ethMethods, | ||
...manager.getMethods(true, rpcDebug !== 'false' && rpcDebug !== ''), | ||
} | ||
break | ||
case MethodConfig.WithoutEngine: | ||
methods = { ...ethMethods } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a default value can be more intuitive than removing it and instead having to handle
undefined
as a value.