-
Notifications
You must be signed in to change notification settings - Fork 111
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
Fix solana-relay logging, fix eslint config #8663
Conversation
|
@@ -1,4 +1,4 @@ | |||
module.exports = { | |||
root: true, | |||
extends: ["custom-server"], |
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.
Change the eslint config
@@ -39,4 +40,4 @@ const main = async () => { | |||
}) | |||
} | |||
|
|||
main().catch(logger.error.bind(logger)) | |||
main().catch((e) => logger.error({ error: e }, 'Fatal error in main!')) |
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.
Added some context here.. not super relevant
) => { | ||
const error = | ||
err instanceof ResponseError ? err : new InternalServerError(String(err)) | ||
if (!res.writableEnded) { | ||
if (!res.headersSent) { |
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.
Changed this (not really necessary, but slightly better)
res: Response | ||
_req: Request, | ||
res: Response, | ||
_next: NextFunction |
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.
Main change
const statusCode = response.statusCode | ||
response.locals.logger.info({ responseTime, statusCode }, 'request completed') | ||
response.locals.logger.info({ requestTime, statusCode }, 'request completed') |
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.
Since this is measuring how long the request takes (not how long to get a response), I renamed this var
@@ -86,8 +90,10 @@ export const relay = async ( | |||
logger | |||
}) | |||
res.status(200).send({ signature }) | |||
next() | |||
const responseTime = new Date().getTime() - res.locals.requestStartTime | |||
logger.info({ responseTime, statusCode: res.statusCode }, 'response sent') |
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.
Added this log to see how long the response took
await broadcastTransaction({ logger, signature }) | ||
next() |
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.
Moved this to prevent duplicate next() calls if broadcastTransaction throws
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.
thank you!
[7343561] [PAY-2842][PAY-2780][PAY-3069] Fix authorized endpoints not supporting manager mode (#8646) Randy Schott [ddaa1b3]⚠️ Third Party Wallet Support [PAY-2949][PAY-2948][PAY-2950] (#8611) Marcus Pasell [c15f20d] Fix prod mediorum dockerfile to have keyfinder dependency (#8666) Michelle Brier [7fe9451] Fix solana-relay logging, fix eslint config (#8663) Marcus Pasell [a495f42] [C-4453] Fix notification email image and layout (#8647) Isaac Solo [2ceae28] Audio analysis: mediorum changes (#8536) Michelle Brier [15a34b3] [C-4353] Add recent searches to search v2 (#8615) Sebastian Klingler [0050b29] Fix sdk typecheck (#8656) Sebastian Klingler
Description
Solana relay logs would log
[Object object]
on failures because the error handling middleware neglected to have the fourth parameter. Annoyingly, when I added the fourth (unused) parameter, eslint was complaining (even with an_
). I realized we were using a different eslint config than client, so I changed that. Which cascaded to a bunch of formatting changes (sorry).I'll annotate the differences to help with review