Skip to content
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

If a response is an error, log it before sending it #417

Closed
hug-dev opened this issue May 7, 2021 · 3 comments
Closed

If a response is an error, log it before sending it #417

hug-dev opened this issue May 7, 2021 · 3 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers small Effort label

Comments

@hug-dev
Copy link
Member

hug-dev commented May 7, 2021

It happens that sometimes, a request errors out and we send the response without having an ERROR log. To catch all possible errors, we could check in the handle_request function in the front end handler if the response we are about to send is an error or not.

if response.header.status != Success {
    error!("Sending back an error: {}", response.header.status);
}

or something similar.

@hug-dev hug-dev added bug Something isn't working good first issue Good for newcomers labels May 7, 2021
@ionut-arm
Copy link
Member

Are you suggesting replacing the other error messages we raise in the providers with this one? Or just adding this on top of the others we already have?

I think it's good to keep the ones we have mostly because it's more easily apparent where they happened. We should maybe review all the paths and try to add logs where we miss them, but a catch-all would also be good.

That log message is also not that comprehensive - we don't copy across the old error when we convert to ResponseStatus, so we just end up not knowing why we got it in the first place.

@hug-dev
Copy link
Member Author

hug-dev commented May 7, 2021

Or just adding this on top of the others we already have?

Yes, just adding on top of the others, in case we don't log it earlier on.

@ionut-arm ionut-arm added the small Effort label label Jul 6, 2021
@MattDavis00
Copy link
Member

I'll work on this

MattDavis00 added a commit to MattDavis00/parsec that referenced this issue Jul 7, 2021
handle_request function.

An ERROR will now be logged before the error response is sent to the
client. Issue parallaxsecond#417.

Signed-off-by: Matt Davis <[email protected]>
ionut-arm added a commit that referenced this issue Jul 8, 2021
#417 Added additional error logging to front end handle_request function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers small Effort label
Projects
None yet
Development

No branches or pull requests

3 participants