foglcz / JSONRpc2

Generic JSON-RPC v2 implementation
Other
19 stars 10 forks source link

Remove of throwing exception inside processing of exceptions throwed by handlers on server side. #13

Closed hobrasoft-cz closed 8 years ago

hobrasoft-cz commented 8 years ago

Throwing exception at this point is quite nonsense as this is the place, where exceptions thrown by the server handlers have to be processed into JSON error response. So when something happens in the handler and it threw out exception about it, it is caught by this try { } catch block and in place of processing it into JSON response, there is thrown out another exception without any relation to original one. This new exception is useless on client side, as you cannot find out what happened on server side and effectively hide the reason of failure. So I propose by this pull request to remove throwing of this new exception and let run error processing until the proper end toward to JSON error response.

scottchiefbaker commented 8 years ago

Interesting... I've fixed this bug (I believe) on my branch here:

https://github.com/scottchiefbaker/php-JSON-RPC/blob/master/lib/Server.php#L271

I need to merge that in to this repo. This specific line is part of check-in 29ee6f89.

hobrasoft-cz commented 8 years ago

Ah, that is better solution of this problem. I will appreciate if you merge that change into master branch soon.

scottchiefbaker commented 8 years ago

I believe I've pushed the changes to this repo. Let me know if this fixes it for you.

scottchiefbaker commented 8 years ago

@hobrasoft-cz one other thing of note... as part of this big merge I landed GET requests as an optional (not enabled by default) style of query. Sometimes better for CLI/curl requests.