- 
                Notifications
    
You must be signed in to change notification settings  - Fork 53
 
Add location data / line numbers to faults #584
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
Conversation
updated logUnhandledFault to also print the location. added context to allclasses that throws faults and were straight forward to add them to. added some TODOs for: TypeMismatch logging in RequestResponseProcess does not print location. ScopeProcess does not add context when an abitrary Excpetion is thrown. OutputPort does not add context for when the following exceptions are turned into Faults: ( ExecutionException | InterruptedException | IOException ) LocalCommChannel needs to handle sending faults differently, as to not overwrite location data. All places that creates FaultException has been reviewed, and there should be no missing changes or TODOs.
…services that communicate over Local
| 
           Nice work! Two questions: 
 I'm slightly concerned about making FaultException mutable with   | 
    
          
 Yes it should, it was a leftover from when I tested with a list of all contexts that the fault would could get, I'll change it. 
 Sadly not, this is done to avoid an embedded service show the same fault line as the embedder. Setting the context to null in fault will still cause the same issue. The issue being that the embedded and the embedder points to the same fault. 
 I feel this would be overengineering, as I see no reason to create a new object when we want to add the context, it could be useful if we opted to handle the local communication issue outside of   | 
    
… new object, making FaultExceptions immutable again
| 
           FaultException are now immutable again by using  However there is now more overhead, as FaultExceptions are created just to create a nearly identical FaultException.  | 
    
| 
           How many constructors are we talking about? If it's like adding 2 to 4 constructors, I'd do it.  | 
    
          
 Short of it 6 new constructors. There exists 6 which all potentially would be duplicated, and from my quick look through it was 5 or 6 needed, and then nearly all   | 
    
…ces of the use of withContext to use a contructor instead, when available.
Pull request for adding location data / line number to faults (see #514)
I've combed over every place that creates a fault and all the places that I could find that forwards them.
I've tested the following manually and seen that they report line numbers(nearly correctly see #582):
There exists some exception that can come up in ScopeProcess and OutputPort which I've not found ways to test.