-
Notifications
You must be signed in to change notification settings - Fork 2
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
SI-2471-burn-vehicle #271
SI-2471-burn-vehicle #271
Conversation
Do we have a handler for |
Co-authored-by: Kevin Joiner <[email protected]>
Co-authored-by: Kevin Joiner <[email protected]>
Co-authored-by: Kevin Joiner <[email protected]>
Co-authored-by: Kevin Joiner <[email protected]>
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.
I didn't finish looking at this, but I took a first glance. I think 424 makes sense as a return code.
logger.Err(err).Msg("database error retrieving nft") | ||
return fiber.NewError(fiber.StatusInternalServerError, "database error retrieving nft") |
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.
The error handler will log on our behalf unless we explicitly call helpers.SkipErrorLog(c)
which we do not.
A good general guideline for most Go functions is that you do not want to log and return an error. Since a function can never guarantee who will be calling it, that function does not know if the caller wants the errors logged or not, so it's best to just return the error and let the caller decide what to do with it.
A situation where you would log and return is if the error contains sensitive information you do not want to give to the calling function. In that situation it may be acceptable to log a detail error message and then return a general message to the user. An example of this is when APIs returning a general 500 Internal Error, but log more specific details.
udOwner.Delete("/", userDeviceController.DeleteUserDevice)
; check if device has been minted; if yes, return an error (StatusFailedDependency, 424) and instruct the user to burn the deviceNOTES:
/commands
in the burn endpoint path? This is consistent with what we have elsewhere but it could also make sense to make the path/v1/vehicle/:tokenID/burn