search

facebookarchive / swift

An annotation-based Java library for creating Thrift serializable types and services.
Apache License 2.0
900 stars 297 forks source link

Exception fixes and one-way method support #15

Closed andrewcox closed 12 years ago

andrewcox commented 12 years ago

Ensures servers properly throw (and clients catch) TApplicationException in the case of unexpected exceptions on the server.

Also adds one-way method call support.

dain commented 12 years ago

looks good. I tried rebasing this on master and when I do I get test failures. Can you rebase and resubmit the pull request?

dain commented 12 years ago

Here is the error I get:

14:59:59.535 [main] INFO c.f.nifty.core.NettyServerTransport - stopping transport thrift:52376 14:59:59.536 [main] INFO c.facebook.nifty.core.NiftyBootstrap - waiting for dispatcher to shutdown org.apache.thrift.TApplicationException: verifyConnectionState failed: out of sequence response at com.facebook.swift.service.ThriftMethodHandler.waitForResponse(ThriftMethodHandler.java:211) at com.facebook.swift.service.ThriftMethodHandler.invoke(ThriftMethodHandler.java:117) at com.facebook.swift.service.ThriftClientManager$ThriftInvocationHandler.invoke(ThriftClientManager.java:258) at $Proxy20.verifyConnectionState(Unknown Source) at com.facebook.swift.service.oneway.TestSuite.testOnewayCall(TestSuite.java:32) 14:59:59.559 [main] INFO c.f.nifty.core.NettyServerTransport - starting transport thrift:52379

andrewcox commented 12 years ago

Yeah, was passing tests for me the last time I looked at it, but I'll definitely take a look again before merging the pull.

From: Dain Sundstrom notifications@github.com<mailto:notifications@github.com> Reply-To: facebook/swift reply@reply.github.com<mailto:reply@reply.github.com> Date: Tue, 21 Aug 2012 15:02:23 -0700 To: facebook/swift swift@noreply.github.com<mailto:swift@noreply.github.com> Cc: Andrew Cox andrewcox@fb.com<mailto:andrewcox@fb.com> Subject: Re: [swift] Exception fixes and one-way method support (#15)

looks good. I tried rebasing this on master and when I do I get test failures. Can you rebase and resubmit the pull request?

— Reply to this email directly or view it on GitHubhttps://github.com/facebook/swift/pull/15#issuecomment-7918014.

andrewcox commented 12 years ago

Breakage was because somewhere in merging stuff, I lost the key condition that keeps servers from sending a response to one-way calls. Checking sequence ids catches this, but there was nothing before that would have caught it.

I'll also add a commit to this pull to fix it, and another commit with some extra checks including the method name check, which would have caught this w/o checking sequence ids.

dain commented 12 years ago

Rebased on master and merged. There were some merge conflicts, so please verify.

andrewcox commented 12 years ago

Verified, thanks!