Exceptions are not being flushed from the server to the client

[rsgoncalves]

From my tests this is happening with exceptions in general. The server throws some specific exception, but the client only gets a general "server internal exception 500", with no information about what went wrong. For example, as guest I tried to commit some changes and got the window in the screenshot below within Protege.

As a user, I've no idea what to do with this. The server reported the actual exception, which was:

14:29:09 [XNIO-2 task-6] ERROR - [Request from UserIdImpl{id=guest} (NameImpl{name=Guest User}) - NameImpl{name=Add axiom}] Request rejected. Operations are not allowed
14:29:09 [XNIO-2 task-6] ERROR - UT005071: Undertow request failed HttpServerExchange{ POST /nci_protege/commit request {Connection=[Keep-Alive], Authorization=[Basic Z3Vlc3Q6MzZhNGZkODYtZTgxNC00ZDdhLWI3MjktNDQ1MTdiNjk3NTA2], Accept-Encoding=[gzip], Content-Length=[46976], User-Agent=[okhttp/3.0.0-RC1], Host=[localhost:8080]} response {}}
org.protege.editor.owl.server.api.exception.ServerServiceException: User has no permission for 'Add axiom' operation
    at org.protege.editor.owl.server.policy.AccessControlFilter.evaluateCommitBundle(AccessControlFilter.java:244) ~[org.protege.owl.server.jar:na]
    at org.protege.editor.owl.server.policy.AccessControlFilter.commit(AccessControlFilter.java:214) ~[org.protege.owl.server.jar:na]
    at org.protege.editor.owl.server.http.handlers.HTTPChangeService.handleRequest(HTTPChangeService.java:50) ~[org.protege.owl.server.jar:na]
    at io.undertow.server.Connectors.executeRootHandler(Connectors.java:198) ~[org.protege.owl.server.jar:na]
    at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:788) ~[org.protege.owl.server.jar:na]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) ~[na:1.8.0_74]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) ~[na:1.8.0_74]
    at java.lang.Thread.run(Thread.java:745) ~[na:1.8.0_74]
Caused by: org.protege.editor.owl.server.api.exception.OperationNotAllowedException: User has no permission for 'Add axiom' operation
    at org.protege.editor.owl.server.api.exception.OperationNotAllowedException.create(OperationNotAllowedException.java:34) ~[org.protege.owl.server.jar:na]
    at org.protege.editor.owl.server.policy.AccessControlFilter.evaluateCommitBundle(AccessControlFilter.java:242) ~[org.protege.owl.server.jar:na]
    ... 7 common frames omitted

[bdionne]

When you say in general do you mean with respect to commits or other operations as well? Which others are presenting problems?

I haven't really worked out the best way to materialize all these exceptions, but the lion's share of the fine-grained metaproject editing methods have been moved to the client, so exceptions there should be wrapped in ClientRequestException and you should be able to access the causes.

I've surfaced the ServerServiceExceptions that are thrown on commits

[johardi]

I'd like to propose a way to propagate exception from server to client. The idea is like this:

1. All server methods must throw the exceptions, rather than just to catch them.
2. All these exceptions must be wrapped in ServerException. It is mandatory the ServerException contains the StatusCode and the error message. (See https://en.wikipedia.org/wiki/List_of_HTTP_status_codes for each status code). To include the cause exception is optional (imo, it's better to flush it using the server's logger)
3. When the exception reaches the handlers, convert it as a response header (e.g., like this).
4. And then when finally the response message reaches the client (i.e., LocalHttpClient), implement the exception handler based on the StatusCode (e.g., like this)

There are two benefits with this approach:

1. It's lightweight. The server doesn't have to serialize and transmit the exception object. The exception is encoded by the StatusCode and the message.
2. It's module independent. Since the message passing is using string, the client and server are very much independent.

Let me know what do you think?

[bdionne]

+1 - I think this is a good approach, simple. We'll have to map the existing exceptions to reasonable HTTP codes, and I think rework some of the legacy objects from the RMI implementation. For example in the changes handler where we call commit, it throws three exceptions though in practice it only throws ServerServiceException which will embed one of the other two. Passing this to the client and then unbundling it there to determine which to throw up to the GUI is awkward, and as you note the GUI only needs to put the error message into a dialog.

[bdionne]

Let me just reopen this until I make all the edit tab changes, which I fully expected would be needed, We dropped the snapshot put for some reason (see #27)

[bdionne]

these all look good to me now - closing this