Community
Participate
Working Groups
Build Identifier: 20110301-1815 The code in getPatchSetDetail(final PatchSet.Id id, IProgressMonitor monitor) assumes that if an exception is thrown from getChangeDetailService().patchSetDetail(id, this); that this must mean that the Gerrit version is < 2.1.7. It then goes on to try the fallback of getChangeDetailService().patchSetDetail(id, this); For me, the second call throws an exception, because I'm trying to make the connector work with Gerrit 2.2.1. I'm clearly getting an "Error parsing request", but I just get exactly what I would expect from this code (given I'm using the compiled jars from 2.2.1): java.lang.NoSuchMethodError: org.eclipse.mylyn.internal.gerrit.core.client.compat.ChangeDetailService.patchSetDetail(Lcom/google/gerrit/reviewdb/PatchSet$Id;Lcom/google/gwt/user/client/rpc/AsyncCallback;)V I Because there is no further catch, this exception is propogated, and so I've lost the original exception that happened on: patchSetDetail(id, null, null, this). I'm not sure what the solution is. Maybe catch NoSuchMethodError and throw the original exception (e) instead. Reproducible: Always Steps to Reproduce: Details of code examination given. Otherwise: run against Gerrit 2.2.1, using the compiled prettify, common and reviewdb jars from 2.2.1
Sascha, do you have an idea how to improve the error handling? Would be nice to make the connector work with 2.2.1 as well :).
If I understood you correctly you're trying to get the connector working with Gerrit 2.2.1 and also compile against the Gerrit 2.2.1 jars. Is there any reason why you do the latter, doesn't it work with the 2.1.5 jars? Would be nice if you could share your plans with us. Anything you would like to contribute? Currently, the connector doesn't rely on functionality from Gerrit > 2.1.5. The Gerrit people know about our "misuse" of the JSON-RPC communication layer (it's actually not an official API) and try to avoid incompatible changes in the future. Actually, if we changed our policy to support only 2.2.1 or greater we could remove the compatibility interface from our coding again. But I wouldn't like do that unless there's a good reason.
Thanks for the quick response. The reason I tried to compile against the 2.2.1 jars is that there has been a change between 2.1.7 and 2.2.1 in the JSON "API". In PatchSetPublishDetail, "given" has been changed from a Map to a List (plus some other changes). This causes a parse error when parsing into the objects in the 2.1.5 libraries. I don't have a huge amount of time to spend working on this, but I wouldn't hesitate to contribute my work back. I just wanted to get it running on Gerrit 2.2.1. I can see the difficulties writing against and unofficial, moving API. I did actually notice some of Shawn Pearce's commit comments referred to maintaining backwards compatibility for you guys. I don't really know the details of the compatibility layer you describe - from what I see you've overridden some of the classes, providing overloads of some of the methods. I'm not sure if that works in all cases (I'm not very familiar with JSON rpc), such as the example I give above. I don't know how you feel about abandoning 2.1.5 support in favour of 2.2.1. 2.2.1 is not yet in "stable", and I'm not sure what Shawn's plans are for release (though I'm using it comfortably, and it seems 'ready'). That said, the Gerrit Connector is still in incubation (in fact, is the whole of Mylyn 3.6 in incubation?), I could see Gerrit 2.2.1 being release before the connector. I suppose the other option is to branch the connector. I don't know how Eclipse handles branches - would the updater support two concurrent branches? I don't know, just throwing that in the pot. Anyway, don't want to step on toes. I just though this connector (and the Hudson/Jenkins connector) were really cool, but to my dismay, only of limited use with Gerrit 2.2.1. Serves me right for upgrading too quickly. Thanks again. Martin
As long as the 2.1.x branch is officially supported by the Gerrit project I would like to maintain backwards compatibility. Still, it would be nice to also support the 2.2.x stream. Do you know if that's possible through overloading additional methods?
I personally suspect it can't be done by overloading methods, as it's the deserialisation of a class that is failing. Existing fields in a class have been changed from a Map to a List - the parser baulks at this. It might need a branch :(
*** Bug 349899 has been marked as a duplicate of this bug. ***
Martin, which version were you trying? I can't actually reproduce the error with http://egit.eclipse.org/r/ which is supposedly "Powered by Gerrit Code Review (2.2.1-67-g17ca55f)".
Given the comments above, and the linked report that has been closed, I think it's probably best if I give the details for the original issue, while using the 2.1.5 in Eclipse. It looks like eclipse must be on a VERY recent version. Mine is: v2.2.1-1-gec40063 which was committed on 2011-06-08. I think 2.2.1-67-g17ca55f must be on an eclipse feature branch, as I can't find it in the google repository (probably branched for branching etc). To reproduce: 1. create a review. 2. open the review in mylyn 3. add yourself as a reviewer, either in Mylyn or Gerrit 4. Save, and resync the review in Mylyn Result: "Unexpected error while communicating with Gerrit" org.eclipse.mylyn.internal.gerrit.core.client.GerritException at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.execute(GerritClient.java:684) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.getPatchSetPublishDetail(GerritClient.java:383) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.getChange(GerritClient.java:400) at org.eclipse.mylyn.internal.gerrit.core.GerritTaskDataHandler.getTaskData(GerritTaskDataHandler.java:81) at org.eclipse.mylyn.internal.gerrit.core.GerritConnector.getTaskData(GerritConnector.java:123) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.synchronizeTask(SynchronizeTasksJob.java:245) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.runInternal(SynchronizeTasksJob.java:218) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.run(SynchronizeTasksJob.java:153) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.run(SynchronizeTasksJob.java:129) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) Caused by: com.google.gson.JsonParseException: The JsonDeserializer com.google.gwtjsonrpc.server.MapDeserializer@184b826 failed to deserialized json object [{"key":{"patchSetId":{"changeId":{"id":527},"patchSetId":1},"accountId":{"id":1000000},"categoryId":{"id":"CRVW"}},"value":0,"granted":"2011-06-14 14:34:37.930000000","changeOpen":true}] given the type com.google.gson.ParameterizedTypeImpl@1daced8 at com.google.gson.JsonDeserializerExceptionWrapper.deserialize(JsonDeserializerExceptionWrapper.java:63) at com.google.gson.JsonDeserializationVisitor.invokeCustomDeserializer(JsonDeserializationVisitor.java:88) at com.google.gson.JsonObjectDeserializationVisitor.visitFieldUsingCustomHandler(JsonObjectDeserializationVisitor.java:116) at com.google.gson.ObjectNavigator.navigateClassFields(ObjectNavigator.java:158) at com.google.gson.ObjectNavigator.accept(ObjectNavigator.java:131) at com.google.gson.JsonDeserializationContextDefault.fromJsonObject(JsonDeserializationContextDefault.java:73) at com.google.gson.JsonDeserializationContextDefault.deserialize(JsonDeserializationContextDefault.java:51) at com.google.gson.Gson.fromJson(Gson.java:568) at org.eclipse.mylyn.internal.gerrit.core.client.JSonSupport.parseResponse(JSonSupport.java:157) at org.eclipse.mylyn.internal.gerrit.core.client.GerritService.invoke(GerritService.java:102) at $Proxy19.patchSetPublishDetail(Unknown Source) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient$9.execute(GerritClient.java:386) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.execute(GerritClient.java:678) ... 9 more Caused by: java.lang.IndexOutOfBoundsException: Index: 1, Size: 1 at java.util.ArrayList.RangeCheck(ArrayList.java:547) at java.util.ArrayList.get(ArrayList.java:322) at com.google.gson.JsonArray.get(JsonArray.java:101) at com.google.gwtjsonrpc.server.MapDeserializer.deserialize(MapDeserializer.java:69) at com.google.gwtjsonrpc.server.MapDeserializer.deserialize(MapDeserializer.java:33) at com.google.gson.JsonDeserializerExceptionWrapper.deserialize(JsonDeserializerExceptionWrapper.java:50) ... 21 more To save you trolling the code - this is basically an issue parsing the newer List used in the 2.2.1 version of PatchSetPublishDetail (and consequently in the JSON), into the 2.1.5 implementation, which uses Maps. I'm having trouble pulling from the Google repo at work, so I can't see if this has been changed in a more recent version. Martin
Ok, managed to pull from google. That commit is definitely not in Google's repo. I can't see anything else that would effect it, so it should be reproducable on egit. Note that if you are not the reviewer, then it works ok. But you cannot actually complete a review.
And I've just connected to http://egit.eclipse.org/r/ and reproduced this. Martin
*** Bug 351021 has been marked as a duplicate of this bug. ***
*** Bug 351666 has been marked as a duplicate of this bug. ***
Is there a target completion for this issue?
We don't have a target release, yet, but I have the task to the backlog to consider it during planning.
I have added work-arounds to support parsing the change permission data fields in Gerrit 2.2: http://review.mylyn.org/#change,26.
Created attachment 203056 [details] mylyn/context/zip
The parsing error is unfortunately not yet fixed. Updated stack trace when opening http://egit.eclipse.org/r/#change,2609: !ENTRY org.eclipse.mylyn.gerrit.core 4 0 2011-09-11 14:07:35.829 !MESSAGE Unexpected error while communicating with Gerrit !STACK 0 org.eclipse.mylyn.internal.gerrit.core.client.GerritException at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.execute(GerritClient.java:770) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.getPatchSetPublishDetail(GerritClient.java:442) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.getChange(GerritClient.java:473) at org.eclipse.mylyn.internal.gerrit.core.GerritTaskDataHandler.getTaskData(GerritTaskDataHandler.java:90) at org.eclipse.mylyn.internal.gerrit.core.GerritConnector.getTaskData(GerritConnector.java:126) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.synchronizeTask(SynchronizeTasksJob.java:245) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.runInternal(SynchronizeTasksJob.java:218) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.run(SynchronizeTasksJob.java:153) at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.run(SynchronizeTasksJob.java:129) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) Caused by: com.google.gson.JsonParseException: The JsonDeserializer com.google.gwtjsonrpc.server.MapDeserializer@56636a77 failed to deserialized json object [{"key":{"patchSetId":{"changeId":{"id":2609},"patchSetId":6},"accountId":{"id":83},"categoryId":{"id":"IPCL"}},"value":0,"granted":"2011-09-09 19:21:16.000000000","changeOpen":true}] given the type com.google.gson.ParameterizedTypeImpl@a0741aba at com.google.gson.JsonDeserializerExceptionWrapper.deserialize(JsonDeserializerExceptionWrapper.java:63) at com.google.gson.JsonDeserializationVisitor.invokeCustomDeserializer(JsonDeserializationVisitor.java:88) at com.google.gson.JsonObjectDeserializationVisitor.visitFieldUsingCustomHandler(JsonObjectDeserializationVisitor.java:116) at com.google.gson.ObjectNavigator.navigateClassFields(ObjectNavigator.java:158) at com.google.gson.ObjectNavigator.accept(ObjectNavigator.java:131) at com.google.gson.JsonDeserializationContextDefault.fromJsonObject(JsonDeserializationContextDefault.java:73) at com.google.gson.JsonDeserializationContextDefault.deserialize(JsonDeserializationContextDefault.java:51) at com.google.gson.Gson.fromJson(Gson.java:568) at org.eclipse.mylyn.internal.gerrit.core.client.JSonSupport.parseResponse(JSonSupport.java:176) at org.eclipse.mylyn.internal.gerrit.core.client.GerritService.invoke(GerritService.java:106) at $Proxy5.patchSetPublishDetailX(Unknown Source) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient$11.execute(GerritClient.java:446) at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.execute(GerritClient.java:764) ... 9 more Caused by: java.lang.IndexOutOfBoundsException: Index: 1, Size: 1 at java.util.ArrayList.RangeCheck(ArrayList.java:547) at java.util.ArrayList.get(ArrayList.java:322) at com.google.gson.JsonArray.get(JsonArray.java:101) at com.google.gwtjsonrpc.server.MapDeserializer.deserialize(MapDeserializer.java:69) at com.google.gwtjsonrpc.server.MapDeserializer.deserialize(MapDeserializer.java:33) at com.google.gson.JsonDeserializerExceptionWrapper.deserialize(JsonDeserializerExceptionWrapper.java:50) ... 21 more
Pushed 862a05dd760c9dc27d1231824ea00a8b5085caac. The connector now registers a custom deserializer to handle the changes in the JSon stream.
*** Bug 361485 has been marked as a duplicate of this bug. ***
*** Bug 361486 has been marked as a duplicate of this bug. ***
