-
Notifications
You must be signed in to change notification settings - Fork 124
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
[IND-458]: In case of errors in comlink log request #722
Conversation
IND-458 In case of errors in comlink log request
When we encounter errors in comlink, it is very useful to get access to the request that threw an error in the indexer to debug. There shouldn't be any PII in comlink since the indexer is a read only service. |
WalkthroughThe changes primarily involve the addition of the Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (16)
- indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts (2 hunks)
- indexer/services/comlink/src/controllers/api/v4/asset-positions-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/candles-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/fills-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/height-controller.ts (2 hunks)
- indexer/services/comlink/src/controllers/api/v4/historical-funding-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/orderbook-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/orders-controller.ts (2 hunks)
- indexer/services/comlink/src/controllers/api/v4/perpetual-markets-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/sparklines-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/trades-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/transfers-controller.ts (1 hunks)
- indexer/services/comlink/src/lib/helpers.ts (4 hunks)
Files skipped from review due to trivial changes (12)
- indexer/services/comlink/src/controllers/api/v4/asset-positions-controller.ts
- indexer/services/comlink/src/controllers/api/v4/candles-controller.ts
- indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts
- indexer/services/comlink/src/controllers/api/v4/height-controller.ts
- indexer/services/comlink/src/controllers/api/v4/historical-funding-controller.ts
- indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts
- indexer/services/comlink/src/controllers/api/v4/orderbook-controller.ts
- indexer/services/comlink/src/controllers/api/v4/orders-controller.ts
- indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts
- indexer/services/comlink/src/controllers/api/v4/sparklines-controller.ts
- indexer/services/comlink/src/controllers/api/v4/trades-controller.ts
- indexer/services/comlink/src/controllers/api/v4/transfers-controller.ts
Additional comments: 8
indexer/services/comlink/src/controllers/api/v4/perpetual-markets-controller.ts (1)
- 142-147: The addition of the
req
parameter to thehandleControllerError
function call is a good practice. It will provide more context about the request that led to the error, which can be useful for debugging. Ensure that thehandleControllerError
function has been updated to handle this new parameter appropriately.indexer/services/comlink/src/controllers/api/v4/fills-controller.ts (1)
- 161-166: The
handleControllerError
function now includes thereq
parameter. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that thehandleControllerError
function correctly handles thereq
parameter and does not expose sensitive information from the request object in the error logs.indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts (2)
269-274: The
req
object is now being passed to thehandleControllerError
function. This will provide more context when logging errors. Ensure that thehandleControllerError
function is properly handling thereq
object.309-314: The
req
object is now being passed to thehandleControllerError
function. This will provide more context when logging errors. Ensure that thehandleControllerError
function is properly handling thereq
object.indexer/services/comlink/src/lib/helpers.ts (4)
48-53: The
handleControllerError
function now accepts an additionalreq
parameter of typeexpress.Request
. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.58-63: The
handleControllerError
function now includes thereq
parameter in its call tohandleInternalServerError
. Ensure that all calls tohandleControllerError
are updated to include thereq
parameter.67-72: The
handleInternalServerError
function now accepts an additionalreq
parameter of typeexpress.Request
. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.79-84: The
handleInternalServerError
function now includes theparams
andquery
properties ofreq
in the error log. This is a good practice as it provides more context about the request that led to the error.
Changelist
Log request in comlink in case of errors
Test Plan
Verifies that the query and parameters correct print out
Author/Reviewer Checklist
state-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.