Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.3
    • Component/s: None
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      This allows javascript clients to be built for browser based access to thrift services.

      The supported transport is XHTTPRequest and the supported Protocol is compatible with TJSONProtocol

      I've added a test in lib/js that is a java based HTTP server which implements ThriftTest service.
      I've put a version of this test service here http://3.rdrail.net:8080/test/test.html if you'd like to see it in action.

      It's been tested in ie 6,7,8, FF, and Safari.

      Let me know if you hit any issues. The protocol and transport code aren't very error tolerant, but overall it seems to work quite well.

      1. thrift_js.patch
        104 kB
        T Jake Luciani
      2. thrift_js_v2.patch
        86 kB
        T Jake Luciani
      3. thrift_js_v3.patch
        87 kB
        T Jake Luciani
      4. THRIFT-550_initialize_with_null.patch
        0.6 kB
        Roger Meier

        Activity

        T Jake Luciani created issue -
        T Jake Luciani made changes -
        Field Original Value New Value
        Attachment thrift_js.patch [ 12414532 ]
        Hide
        T Jake Luciani added a comment -

        Next iteration of patch.

        • Fixed namespaces and updated tests.
        • Removed xfer info and useless semicolons
        Show
        T Jake Luciani added a comment - Next iteration of patch. Fixed namespaces and updated tests. Removed xfer info and useless semicolons
        T Jake Luciani made changes -
        Attachment thrift_js_v2.patch [ 12417949 ]
        Hide
        T Jake Luciani added a comment -

        Latest version. Includes misc bug fixes and added a async example in demo.

        Take a look, I'll commit this in next couple days if no complaints.

        Show
        T Jake Luciani added a comment - Latest version. Includes misc bug fixes and added a async example in demo. Take a look, I'll commit this in next couple days if no complaints.
        T Jake Luciani made changes -
        Attachment thrift_js_v3.patch [ 12435758 ]
        Hide
        T Jake Luciani added a comment -

        commited

        Show
        T Jake Luciani added a comment - commited
        T Jake Luciani made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 0.3 [ 12314451 ]
        Resolution Fixed [ 1 ]
        Hide
        Roger Meier added a comment -

        I had a problem with the exception avaliable on the tutorial.

        i32 calculate(1:i32 logid, 2:Work w) throws (1:InvalidOperation ouch),

        It couldn't be thrown because initialization of numeric base types is done with 0 instead of null and the checks do compare against null.
        This was not visible with the Tests provided with first patch for JavaScript bindings above, the ThriftTest.thrift definition does not have a combination of a base type return value and an exception.

        I've made a patch that initializes the base types I16,I32, I64 and DOUBLE with null. This could probably solve other issues as well

        Regards

        Roger

        Show
        Roger Meier added a comment - I had a problem with the exception avaliable on the tutorial. i32 calculate(1:i32 logid, 2:Work w) throws (1:InvalidOperation ouch), It couldn't be thrown because initialization of numeric base types is done with 0 instead of null and the checks do compare against null. This was not visible with the Tests provided with first patch for JavaScript bindings above, the ThriftTest.thrift definition does not have a combination of a base type return value and an exception. I've made a patch that initializes the base types I16,I32, I64 and DOUBLE with null. This could probably solve other issues as well Regards Roger
        Roger Meier made changes -
        Attachment THRIFT-550_initialize_with_null.patch [ 12447472 ]
        Hide
        David Reiss added a comment -

        Can you open a new issue for this bug so it can be tracked separately?

        Show
        David Reiss added a comment - Can you open a new issue for this bug so it can be tracked separately?
        Hide
        Roger Meier added a comment -

        Sure! I have created a new Bug Report for that issue:
        https://issues.apache.org/jira/browse/THRIFT-807

        Show
        Roger Meier added a comment - Sure! I have created a new Bug Report for that issue: https://issues.apache.org/jira/browse/THRIFT-807
        Hide
        Jordan added a comment - - edited

        Awesome project. You have no idea how helpful this is. I mean, could be quite helpful in the revolution of bring more complex client code to the front end. In addition, I can use the same service for my ajax webpage as I use for my iPhone app. A couple of issues:

        1. Runtime exceptions. I would expect my operations to be able to accidentally throw an NPE/IllegalArgs (in Java) and have my js client catch them as follows:
        catch(run) {

        }
        But it does not work. In some cases, the "run" is "undefined" and in other cases the auto-generated js bugs out.

        Unfortunately it may be more complicated than we think. For service operations in the .thrift file that declare a checked exception (that was also defined in the .thrift file) I notice something in the auto-generated Java code.
        For these operations that declare exceptions, there is a catch(Throwable) which transforms it into a TApplicationException.
        Unfortunately, for service operations that do NOT declare an exception, it does NOT catch throwable which seems very odd to me. Whether or not something declares a checked exception has nothing to do with whether or not it throws runtime exceptions. I believe this is a bug, go see for yourself. Search for "public class yourOperationName implements ProcessFunction".

        2. Now, that aside, I still see some problems on the js side of things.
        For operations that do catch throwable on the client side, the js code tries to initalize a Thrift.ApplicationException.. Which doesn't exist. However there does appear to be something called Thrift.TApplicationException defined in thrift.js. Was this a bug?

        3. I would like a nicer async api (I can show you what I hacked up at the bottom of the post.)

        3. Once this is working, how can we catch exceptions from asynchronous methods? The async api I like would be the following:
        SYNC API:
        client.callMethod(param1, param2);
        ASYNC API:
        asyncClient.callMethod(handler, param1, param2);
        With this api it won't work at all. We would need to supply another error handler. Oh well..

        4. It would be cool to have nicer async clients: Here is some code I came up with that takes in a client and generates an asyncClient with all of the same methods, except that the first parameter is the callback (yes, we'll eventually need an error callback too.) (yes, it requires jQuery, but I think that we could just require that the user have it for async clients.)

        function createAsyncThriftClient(syncThriftClient) {
        var retObj = {};
        for(var methodName in syncThriftClient) {
        if(typeof syncThriftClient[methodName] === 'function') {
        if(methodName.substring(0,5) == "send_" || methodName.substring(0,5) == "recv_")

        { retObj[methodName] = syncThriftClient[methodName]; }

        else

        { retObj[methodName] = createThriftAsyncCall(syncThriftClient, methodName) }

        }
        }
        return retObj;
        }

        var createThriftAsyncCall = function(syncThriftClient, methodName) {
        return function() {
        var calledMethodName = methodName;
        if(arguments.length < 1)

        { throw("Assync call must have a handler"); }

        var successHandler = arguments[arguments.length-1];
        var apiArgs = new Array();
        for(var i = 0; i < arguments.length-1; i++)

        { apiArgs[i] = arguments[i]; }

        jQuery.ajax({
        url: "/service",
        data: syncThriftClient["send_" + calledMethodName].apply(syncThriftClient, apiArgs),
        type: "POST",
        cache: false,
        success: function(res)

        { var _transport = new Thrift.Transport(); var _protocol = new Thrift.Protocol(_transport); var _client = new TheService.TheServiceClient(_protocol); _transport.setRecvBuffer(res); var v = _client["recv_" + calledMethodName].apply(_client); successHandler(v); }

        });
        }
        };

        EDIT: Actually, that code requires the callback as the last parameter to the assync.methodCall().

        Show
        Jordan added a comment - - edited Awesome project. You have no idea how helpful this is. I mean, could be quite helpful in the revolution of bring more complex client code to the front end. In addition, I can use the same service for my ajax webpage as I use for my iPhone app. A couple of issues: 1. Runtime exceptions. I would expect my operations to be able to accidentally throw an NPE/IllegalArgs (in Java) and have my js client catch them as follows: catch(run) { } But it does not work. In some cases, the "run" is "undefined" and in other cases the auto-generated js bugs out. Unfortunately it may be more complicated than we think. For service operations in the .thrift file that declare a checked exception (that was also defined in the .thrift file) I notice something in the auto-generated Java code. For these operations that declare exceptions, there is a catch(Throwable) which transforms it into a TApplicationException. Unfortunately, for service operations that do NOT declare an exception, it does NOT catch throwable which seems very odd to me. Whether or not something declares a checked exception has nothing to do with whether or not it throws runtime exceptions. I believe this is a bug, go see for yourself. Search for "public class yourOperationName implements ProcessFunction". 2. Now, that aside, I still see some problems on the js side of things. For operations that do catch throwable on the client side, the js code tries to initalize a Thrift.ApplicationException.. Which doesn't exist. However there does appear to be something called Thrift.TApplicationException defined in thrift.js. Was this a bug? 3. I would like a nicer async api (I can show you what I hacked up at the bottom of the post.) 3. Once this is working, how can we catch exceptions from asynchronous methods? The async api I like would be the following: SYNC API: client.callMethod(param1, param2); ASYNC API: asyncClient.callMethod(handler, param1, param2); With this api it won't work at all. We would need to supply another error handler. Oh well.. 4. It would be cool to have nicer async clients: Here is some code I came up with that takes in a client and generates an asyncClient with all of the same methods, except that the first parameter is the callback (yes, we'll eventually need an error callback too.) (yes, it requires jQuery, but I think that we could just require that the user have it for async clients.) function createAsyncThriftClient(syncThriftClient) { var retObj = {}; for(var methodName in syncThriftClient) { if(typeof syncThriftClient [methodName] === 'function') { if(methodName.substring(0,5) == "send_" || methodName.substring(0,5) == "recv_") { retObj[methodName] = syncThriftClient[methodName]; } else { retObj[methodName] = createThriftAsyncCall(syncThriftClient, methodName) } } } return retObj; } var createThriftAsyncCall = function(syncThriftClient, methodName) { return function() { var calledMethodName = methodName; if(arguments.length < 1) { throw("Assync call must have a handler"); } var successHandler = arguments [arguments.length-1] ; var apiArgs = new Array(); for(var i = 0; i < arguments.length-1; i++) { apiArgs[i] = arguments[i]; } jQuery.ajax({ url: "/service", data: syncThriftClient ["send_" + calledMethodName] .apply(syncThriftClient, apiArgs), type: "POST", cache: false, success: function(res) { var _transport = new Thrift.Transport(); var _protocol = new Thrift.Protocol(_transport); var _client = new TheService.TheServiceClient(_protocol); _transport.setRecvBuffer(res); var v = _client["recv_" + calledMethodName].apply(_client); successHandler(v); } }); } }; EDIT: Actually, that code requires the callback as the last parameter to the assync.methodCall().
        Hide
        Jordan added a comment - - edited

        UPDATE:
        I just tried throwing a CHECKED exception (that was declared in the .thrift file) from a sync client (which I though was working) and it seems like the exception that was caught is not at all related to the one that was thrown.
        Instead the caught exception is related to "this.robj is undefined."
        I do understand that the test cases are working fine, and those exceptions are being caught properly in the js, but mine are not. I can't find what is different. I practically copied the code and just renamed the Exception and the service operation, yet it won't catch mine.
        What can I do, if anything, to help get this problem fixed. I don't understand much about this project, but it's important that exceptions get translated into javascript correctly.

        Show
        Jordan added a comment - - edited UPDATE: I just tried throwing a CHECKED exception (that was declared in the .thrift file) from a sync client (which I though was working) and it seems like the exception that was caught is not at all related to the one that was thrown. Instead the caught exception is related to "this.robj is undefined." I do understand that the test cases are working fine, and those exceptions are being caught properly in the js, but mine are not. I can't find what is different. I practically copied the code and just renamed the Exception and the service operation, yet it won't catch mine. What can I do, if anything, to help get this problem fixed. I don't understand much about this project, but it's important that exceptions get translated into javascript correctly.
        Hide
        Roger Meier added a comment -

        Hi Jordan,

        I had similar issues with exceptions! That's why I've added a patch to fix this, see above or have a look on the newly created issue:
        https://issues.apache.org/jira/browse/THRIFT-807

        Roger

        Show
        Roger Meier added a comment - Hi Jordan, I had similar issues with exceptions! That's why I've added a patch to fix this, see above or have a look on the newly created issue: https://issues.apache.org/jira/browse/THRIFT-807 Roger
        Hide
        Jordan added a comment -

        Roger,

        I took a look at that. Thank you. I did mention in that Jira item that my operation returns a boolean, not void and not i32, and I asked you in that jira task if your patch will fix that as well. If you could, could you please answer that question in that jira task thread? Thank you.

        Also, supposing that exceptions ARE indeed thrown correctly. What happens with a runtime exception? What do you think SHOULD happen with a runtime exception. It seems from the service side, we should never let an unchecked exception be thrown back to the client. In the java generated code, it masks them as TApplicationException, which is good, but can you verify that the way it does that is broken? Look at an operation that does not declare an exception. Notice how it does not catch throwable and transform to TApplicationException.

        Granted, I think it's wise to declare that all of your operations throw YourMadeUpServiceException.

        Show
        Jordan added a comment - Roger, I took a look at that. Thank you. I did mention in that Jira item that my operation returns a boolean, not void and not i32, and I asked you in that jira task if your patch will fix that as well. If you could, could you please answer that question in that jira task thread? Thank you. Also, supposing that exceptions ARE indeed thrown correctly. What happens with a runtime exception? What do you think SHOULD happen with a runtime exception. It seems from the service side, we should never let an unchecked exception be thrown back to the client. In the java generated code, it masks them as TApplicationException, which is good, but can you verify that the way it does that is broken? Look at an operation that does not declare an exception. Notice how it does not catch throwable and transform to TApplicationException. Granted, I think it's wise to declare that all of your operations throw YourMadeUpServiceException.
        Jake Farrell made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            T Jake Luciani
            Reporter:
            T Jake Luciani
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development