The case that's been highlighted here (user.GetRoles) appears to be the only one that presents a significant risk (i.e. could be invoked when server returns an error, and isn't recovered in all cases). Given the high priority, have moved that to its own ticket (CBG-308)
The rest are either using panic correctly (identifying that the system is in an unrecoverable state, unrelated to a single operation/connection), or intentionally triggering the recover in the rest handler code. The latter cases could be cleaned up, to avoid unintentional misuse in future (as appears to be the case for user.GetRoles).
There are two other cases that could be consider for a switch to error handling, although they also don't appear to be critical
- This has usages that aren't following the documented recommendation (only hardcoded known-valid strings). Instead of removing the panic, may be more appropriate to remove those usages. Based on an initial review, all usages involve channel names that have already been validated (i.e. they exist on docs).
- This doesn't appear to be a particular risk, as it's only being invoked for structs that have previously been successfully unmarshalled by the same library. However, switching to error handling and bubbling the error up through createRevID is probably still preferable.