Last modified: 2013-04-12 18:39:56 UTC
gerrit draft ( git review -D ): I cannot add reviewer with black message "Code Review error: Internal Server Error 500" noticed yesterday, when I used git review -D . Can not add Ryan, cannot add myself as reviewer.... Please fix soon. Important feature, showstopper
(In reply to comment #0) > showstopper Workaround: In case it's not secret, don't make it a draft and add a comment?
My git-review doesn't have that option, but doing directly git push origin HEAD:refs/drafts/master/topic worked fine: https://gerrit.wikimedia.org/r/52780
> Workaround: In case it's not secret, don't make it a draft and add a comment? it is/was a secret
(In reply to comment #2) > My git-review doesn't have that option, but doing directly > git push origin HEAD:refs/drafts/master/topic > worked fine: https://gerrit.wikimedia.org/r/52780 I am not talking about pushing review commits. I am talking about the "Internal Server Error 500" which comes up, when I _then_ add - in the GERRIT Web panel a Reviewer. There is the usual button and input selection for reviewers. but it crashes the server or something.
Could not replicate this usiny my own Gerrit instance running 2.5.2-1365-g7609714. I could push draft change (Status: Draft) and I could add the reviewer successfully. ^demon, could you paste the exception?
BTW: Ryan was able to reproduce that Internal Server Error 500, too.
(In reply to comment #5) > Could not replicate this usiny my own Gerrit instance running > 2.5.2-1365-g7609714. I could push draft change (Status: Draft) and I could > add > the reviewer > successfully. > > ^demon, could you paste the exception? NPE, yay. [2013-03-08 00:24:29,819] ERROR com.google.gerrit.httpd.restapi.RestApiServlet : Error in POST /r/changes/52722/reviewers java.lang.NullPointerException at com.google.gerrit.server.change.ReviewerJson.format(ReviewerJson.java:112) at com.google.gerrit.server.change.PostReviewers.addReviewers(PostReviewers.java:233) at com.google.gerrit.server.change.PostReviewers.putAccount(PostReviewers.java:147) at com.google.gerrit.server.change.PostReviewers.apply(PostReviewers.java:131) at com.google.gerrit.server.change.PostReviewers.apply(PostReviewers.java:62) at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:284) at javax.servlet.http.HttpServlet.service(HttpServlet.java:722) at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:263) at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:178) at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:91) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:62) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:168) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.gwtexpui.server.CacheControlFilter.doFilter(CacheControlFilter.java:70) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:163) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.gerrit.httpd.RequireSslFilter.doFilter(RequireSslFilter.java:68) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:163) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:168) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:168) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.gerrit.httpd.AllRequestFilter$FilterProxy$1.doFilter(AllRequestFilter.java:64) at com.google.gerrit.httpd.AllRequestFilter$FilterProxy.doFilter(AllRequestFilter.java:57) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:163) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.gerrit.httpd.RequestContextFilter.doFilter(RequestContextFilter.java:75) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:163) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:168) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:58) at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:118) at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:113) at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1307) at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:453) at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:229) at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1072) at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:382) at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:193) at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1006) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:135) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:116) at org.eclipse.jetty.server.Server.handle(Server.java:365) at org.eclipse.jetty.server.AbstractHttpConnection.handleRequest(AbstractHttpConnection.java:485) at org.eclipse.jetty.server.AbstractHttpConnection.content(AbstractHttpConnection.java:937) at org.eclipse.jetty.server.AbstractHttpConnection$RequestHandler.content(AbstractHttpConnection.java:998) at org.eclipse.jetty.http.HttpParser.parseNext(HttpParser.java:856) at org.eclipse.jetty.http.HttpParser.parseAvailable(HttpParser.java:240) at org.eclipse.jetty.server.AsyncHttpConnection.handle(AsyncHttpConnection.java:82) at org.eclipse.jetty.io.nio.SelectChannelEndPoint.handle(SelectChannelEndPoint.java:627) at org.eclipse.jetty.io.nio.SelectChannelEndPoint$1.run(SelectChannelEndPoint.java:51) at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:608) at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:543) at java.lang.Thread.run(Thread.java:679)
Confirmed. It also fails from command line: $ ssh gerrit.wikimedia.org -p 29418 gerrit set-reviewers 52915 -a "Wikinaut" error: could not add Wikinaut: null fatal: one or more updates failed; review output above
This is fixed already in gerrit master: https://gerrit-review.googlesource.com/#/c/42850/ commit d50e45658420f8fb25759f09b9bf7ab61923c763 Author: Shawn Pearce <sop@google.com> Date: Fri Mar 1 09:31:44 2013 -0800 Skip SubmitRecords that are missing labels A record cna be missing labels if the change is closed, or the patch set is not the current patch set, etc. Skip these rather than NPE. Change-Id: Iaa5d32ee9a7279a66535c66d3ef6eb05efd3a837
BTW, what does NPE mean?
NullPointerException
Upgraded to 2.5.2-1636-g353e384, which contains the fix in comment 9. Should be FIXED now.
No error, but user does not show (they're actually there, tricky tricky). We need https://gerrit-review.googlesource.com/#/c/43230/.
(In reply to comment #12) > Upgraded to 2.5.2-1636-g353e384, which contains the fix in comment 9. Should > be > FIXED now. It is not fully fixed, because: + I just tested it with https://gerrit.wikimedia.org/r/#/c/53265/ . + I tried to add you as reviewer. + The good message is: there is no NPE anymore. + bad message: I still cannot add a reviewer, tried to add you as reviewer; don't know, if this is foreseen for a DRAFT, but in my view, it is also bug and should be signalled upstream.
BTW, DRAFT has (will have) many advantages especially for new comitters. Why ? Because it allows you to see what you have done locally, to fix white space problems, to abandon selected patchs sets, to rebase -- and when you are confident, that everything looks fine -- then to press "PUBLISH" I like git review -D VERY MUCH. It's the easiest way for newcomers to get acquainted to gerrit and our workflow.
(In reply to comment #13) > No error, but user does not show (they're actually there, tricky tricky). > > We need https://gerrit-review.googlesource.com/#/c/43230/. Ah, I see. We had a mid-air collision, I now do see the MAIL. Chad: can you please do something with that test patch e.g. comment, and a +1 or so ?
I saw (by mail) that you, Chad, has invited Ryan to review. It appears really to work, at least partially, the mail system.
Yeah, I knew it's fixed (like I said, you get added but just don't show up in the UI). Lowering the priority/severity since the NPE is now fixed and there's no behavior problems. We can deploy this patch for the UI fix as a part of our next deploy. I'd rather not roll out two builds in one day, so we'll schedule an update for sometime later in the week.
Chad, can you please actively make a change on that patch, e.g. add a comment ? I am interested to see how that looks like then.
