Skip to content

Audit panics #547

@vinistock

Description

@vinistock

As we developed Rubydex, we didn't pay particularly intense attention to all possible panic cases. Some of them are definitely valid, but we need to audit everything and make sure that we're returning Result<> objects instead when applicable. This is critical because if Rubydex panics, it will crash the LSP (or any other project using it).

Review all panics in our code and, if applicable, update to use results instead. Here are some examples to help guide judgement:

  • Inconsistency in the graph. For example, we get a NameId that was registered as part of a definition, but the name doesn't exist. This is okay to panic because it indicates a bug in our code that we have to fix. Soft failing on data inconsistencies like these will just result in obscure bugs
  • Expecting a specific type (e.g.: declaration.as_namespace().unwrap()). This is okay as long as you're sure that it can only be a namespace under the conditions imposed by the code
  • Panicking on public APIs based on user inputs. These cases are not okay and we need to return result objects. For example: a public API receives a declaration ID and then assumes that the declaration will be a namespace and unwraps. This isn't good because users of the API can just pass whatever declaration ID they want

Metadata

Metadata

Assignees

No one assigned

    Labels

    easyThis issue is relatively easy. It has low ambiguity or technical challenges

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions