spullara / redis-protocol

Java client and server implementation of Redis
356 stars 134 forks source link

ClassCastException on brpoplpush timeout #21

Closed jencompgeek closed 11 years ago

jencompgeek commented 11 years ago

Calling brpoplpush on an empty source list results in a ClassCastException.

RedisClient client = new RedisClient("localhost", 6379);
client.brpoplpush("alist", "foo", 1);

java.lang.ClassCastException: redis.reply.MultiBulkReply cannot be cast to redis.reply.BulkReply
at redis.client.RedisClient.brpoplpush(RedisClient.java:967)
at org.springframework.data.redis.connection.srp.SrpTest.testBrPopLPushTimeout(SrpTest.java:82)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
spullara commented 11 years ago

Basically the machine documentation is wrong in this case because of the exceptional timeout condition. I can fix this one of two ways:

1) special case this operation by moving it to the base class and having it throw an exception on timeout 2) treat it as I do other multi type reply methods and just push it to the end user to check to see if it is a bulk replly or a nil multi-bulk reply

My bias is towards 2) because the more that can be shared amongst all the clients the better.