Improve error handling and presentation

 






 

 


  Review Request #10446
  —
  Created March 16, 2019 and updated 

 




















 

  

   Information

  





 
 

  brennie
 









 
 

  rb-gateway
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  rb-gateway
 









 
 

  
 






 








 





  


 
 

  
Previously almost every error that was generated, where it be by 

rb-gateway itself or a by a support library, was presented to the user

as-is. We had no distinction between a user-visible error (such as when

a requested commit or file does not exist) and an internal error (such

as when the call to hg log fails).


We now make the distinction explicit with two new kinds of errors:


    

  • InternalError, which is visible only in log messages and if
    
  returned in an API response will be a generic HTTP 500, and
    • 

  • UserVisibleError, which must be explicitly opted-in to and should
    
  only contain sanitized information suitable for the end user.
    • 



Both of these errors also support the new ErrorChain interface, which

allows errors to declare a cause, e.g., a user visible error that

branches could not be retrieved would have a cause that is the internal

error from hg.


Error causes are not presently returned in API responses, due to them

mostly being internal, but the entire chain will be shown in log

messages.


A few other cleanups have been made:


    

  • All error responses are now JSON instead of plain text.
    • 

  • Some incorrect json field tags have been corrected.
    • 


 








  


 
 

  
    

  • Built and ran rb-gateway. Observed errors were logged correctly.
    • 

  • Ran unit tests.
    • 

  • Failed a POST to the /session endpoint and got a JSON response.
    • 


 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

    Author

   

  
  

   


    

     

      Remove superfluous "omitempty" from json field tags
      
The `parsedRequest` struct had incorrect json field tags (the correct
tag would be `json:"name,omitempty"`) but `omitempty` is only used when
serializing the structure.

     


    		
    720dcd87ae31b0346ad45ebc2221495bb965d9d2

    Barret Rennie

   


   


    

     

      Add the errors package for creating and chaining errors
      
The `errors` package adds utilities for creating and chaining together
errors, creating traces. The `ErrorChain` interface extends the regular
`error` interface, adding the `Cause` method which will return either
its cause (if any) or `nil`.

This interface is implemented by two new types: `InternalError` and
`UserVisibleError`. The former is for error messages that are only to be
written in the server log, whereas the latter may contain messages to be
presented to an API consumer. All errors constructed via methods in the
`errors` package are by default `InternalError`s and can only become
`UserVisibleErrors` by calling the `UserVisible()` method on
`InternalError`. Runtime type checking can be used to distinguish the
two from eachother, creating a type enforced error boundary.

Additionally, the `errors.LogError` method has been added for logging
error messages. If the given error message implements the `ErrorChain`
interface, the entire chain will be walked and logged.

Testing done:
- Compiled and ran rb-gateway.

     


    		
    545d68aec8c2547dd8d169418b3920066000e441

    Barret Rennie

   


   


    

     

      Add helpers for serializing error messages to JSON
      
Previously, all error messages were written directly into response
bodies. However, our API is JSON based and it therefore makes sense to
also return our errors in a JSON package. This will allow our errors to
be extendable in the future, should serializing more than a message be
required.

Testing done:
- Compile and ran rb-gateway.
- Ran unit tests.

     


    		
    efc7853c33099005d29c180f6bcfadf7a9fd4d97

    Barret Rennie

   


   


    

     

      Use `errors` module everywhere
      
We now use the `errors.New*` methods everywhere we were using the
(stdlib) `errors.New` or `fmt.Error` methods, allowing all errors
originating inside of RB-Gateway to be chainable (instead of just a
message with the previous error message splatted inside of it) and
control their visibility to end users.

Testing done:
- Built and ran rb-gateway. Observed errors were logged correctly.
- Ran unit tests.

     


    		
    f2d385834bdd974f2896aa804763251d5957c063

    Barret Rennie

   


   


    

     

      Return JSON errors when authorization fails
      
When attempting to authorize with the `/session` endpoint, we now return
the error formatted as JSON (through the `NormalHeaders` configuration
provided by `go-http-auth`)

Testing done:
- Built and ran rb-gateway.
- Ran unit tests.
- Failed a POST to the `/session` endpoint and got a JSON response.

     


    		
    7976b34be7cf7ca40b10c0e15cc38f30651d149f

    Barret Rennie

   



  
 




 













 









 






  

 



















 




 




 














 



 

  

   




brennie




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
Use WriteError in api.withRepository.

go fmt

   

  








  

   

    Commits:
   

   

    





 

  
   


    

    Summary
    ID

    Author

   

  
  

   


    

    

     

      Remove superfluous "omitempty" from json field tags
      
The `parsedRequest` struct had incorrect json field tags (the correct
tag would be `json:"name,omitempty"`) but `omitempty` is only used when
serializing the structure.

     


    		
    720dcd87ae31b0346ad45ebc2221495bb965d9d2

    Barret Rennie

   


   


    

    

     

      Add the errors package for creating and chaining errors
      
The `errors` package adds utilities for creating and chaining together
errors, creating traces. The `ErrorChain` interface extends the regular
`error` interface, adding the `Cause` method which will return either
its cause (if any) or `nil`.

This interface is implemented by two new types: `InternalError` and
`UserVisibleError`. The former is for error messages that are only to be
written in the server log, whereas the latter may contain messages to be
presented to an API consumer. All errors constructed via methods in the
`errors` package are by default `InternalError`s and can only become
`UserVisibleErrors` by calling the `UserVisible()` method on
`InternalError`. Runtime type checking can be used to distinguish the
two from eachother, creating a type enforced error boundary.

Additionally, the `errors.LogError` method has been added for logging
error messages. If the given error message implements the `ErrorChain`
interface, the entire chain will be walked and logged.

Testing done:
- Compiled and ran rb-gateway.

     


    		
    545d68aec8c2547dd8d169418b3920066000e441

    Barret Rennie

   


   


    

    

     

      Add helpers for serializing error messages to JSON
      
Previously, all error messages were written directly into response
bodies. However, our API is JSON based and it therefore makes sense to
also return our errors in a JSON package. This will allow our errors to
be extendable in the future, should serializing more than a message be
required.

Testing done:
- Compile and ran rb-gateway.
- Ran unit tests.

     


    		
    efc7853c33099005d29c180f6bcfadf7a9fd4d97

    Barret Rennie

   


   


    

    

     

      Use `errors` module everywhere
      
We now use the `errors.New*` methods everywhere we were using the
(stdlib) `errors.New` or `fmt.Error` methods, allowing all errors
originating inside of RB-Gateway to be chainable (instead of just a
message with the previous error message splatted inside of it) and
control their visibility to end users.

Testing done:
- Built and ran rb-gateway. Observed errors were logged correctly.
- Ran unit tests.

     


    		
    dfd0fb287e28260d6699a1ef2413e76ccc9f30df

    Barret Rennie

   


   


    

    

     

      Return JSON errors when authorization fails
      
When attempting to authorize with the `/session` endpoint, we now return
the error formatted as JSON (through the `NormalHeaders` configuration
provided by `go-http-auth`)

Testing done:
- Built and ran rb-gateway.
- Ran unit tests.
- Failed a POST to the `/session` endpoint and got a JSON response.

     


    		
    74af14eee6ed1059044b4acea9dea190d6b7ace0

    Barret Rennie

   




   


    

    

     

      Remove superfluous "omitempty" from json field tags
      
The `parsedRequest` struct had incorrect json field tags (the correct
tag would be `json:"name,omitempty"`) but `omitempty` is only used when
serializing the structure.

     


    		
    720dcd87ae31b0346ad45ebc2221495bb965d9d2

    Barret Rennie

   


   


    

    

     

      Add the errors package for creating and chaining errors
      
The `errors` package adds utilities for creating and chaining together
errors, creating traces. The `ErrorChain` interface extends the regular
`error` interface, adding the `Cause` method which will return either
its cause (if any) or `nil`.

This interface is implemented by two new types: `InternalError` and
`UserVisibleError`. The former is for error messages that are only to be
written in the server log, whereas the latter may contain messages to be
presented to an API consumer. All errors constructed via methods in the
`errors` package are by default `InternalError`s and can only become
`UserVisibleErrors` by calling the `UserVisible()` method on
`InternalError`. Runtime type checking can be used to distinguish the
two from eachother, creating a type enforced error boundary.

Additionally, the `errors.LogError` method has been added for logging
error messages. If the given error message implements the `ErrorChain`
interface, the entire chain will be walked and logged.

Testing done:
- Compiled and ran rb-gateway.

     


    		
    545d68aec8c2547dd8d169418b3920066000e441

    Barret Rennie

   


   


    

    

     

      Add helpers for serializing error messages to JSON
      
Previously, all error messages were written directly into response
bodies. However, our API is JSON based and it therefore makes sense to
also return our errors in a JSON package. This will allow our errors to
be extendable in the future, should serializing more than a message be
required.

Testing done:
- Compile and ran rb-gateway.
- Ran unit tests.

     


    		
    efc7853c33099005d29c180f6bcfadf7a9fd4d97

    Barret Rennie

   


   


    

    

     

      Use `errors` module everywhere
      
We now use the `errors.New*` methods everywhere we were using the
(stdlib) `errors.New` or `fmt.Error` methods, allowing all errors
originating inside of RB-Gateway to be chainable (instead of just a
message with the previous error message splatted inside of it) and
control their visibility to end users.

Testing done:
- Built and ran rb-gateway. Observed errors were logged correctly.
- Ran unit tests.

     


    		
    f2d385834bdd974f2896aa804763251d5957c063

    Barret Rennie

   


   


    

    

     

      Return JSON errors when authorization fails
      
When attempting to authorize with the `/session` endpoint, we now return
the error formatted as JSON (through the `NormalHeaders` configuration
provided by `go-http-auth`)

Testing done:
- Built and ran rb-gateway.
- Ran unit tests.
- Failed a POST to the `/session` endpoint and got a JSON response.

     


    		
    7976b34be7cf7ca40b10c0e15cc38f30651d149f

    Barret Rennie

   



  
 







   

  





  

   

    Diff:
   

   

    
 Revision 2 (+824 -200)
Show changes
 
    api/api.go 
    api/errors.go 
    api/errors_test.go 
    api/routes.go 
    api/routes_test.go 
    api/tokens/memory_store.go 
    commands/serve.go 
    config/config.go 
  7 more
 

   

  











 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.