github-api icon indicating copy to clipboard operation
github-api copied to clipboard

Bug: Do not try to populate a repo on Installation:deleted event

Open Haarolean opened this issue 2 years ago • 1 comments

Hi, there is an issue in case of trying to parse event payload of type Installation ("deleted"):

The installation at this point has been deleted so fetching repo info is not possible.

The stack trace looks like this:

org.kohsuke.github.GHException: Failed to refresh repositories
	at org.kohsuke.github.GHEventPayload$Installation.lateBind(GHEventPayload.java:298)
	at org.kohsuke.github.GitHub.parseEventPayload(GitHub.java:958)
	at xxx.core.controller.github.GithubController.hook(GithubController.java:61)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:207)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:152)
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:118)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:884)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:797)
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1081)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:974)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1011)
	at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:914)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:590)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:885)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:658)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:205)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
	at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:51)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
	at org.springframework.web.filter.ServerHttpObservationFilter.doFilterInternal(ServerHttpObservationFilter.java:109)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:166)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:482)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:115)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:341)
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:391)
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63)
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:894)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1741)
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: org.kohsuke.github.HttpException: {"message":"Bad credentials","documentation_url":"https://docs.github.com/rest"}
	at org.kohsuke.github.GitHubConnectorResponseErrorHandler$1.onError(GitHubConnectorResponseErrorHandler.java:62)
	at org.kohsuke.github.GitHubClient.detectKnownErrors(GitHubClient.java:473)
	at org.kohsuke.github.GitHubClient.sendRequest(GitHubClient.java:435)
	at org.kohsuke.github.GitHubClient.sendRequest(GitHubClient.java:403)
	at org.kohsuke.github.Requester.fetchInto(Requester.java:102)
	at org.kohsuke.github.GHRepository.populate(GHRepository.java:3509)
	at org.kohsuke.github.GHEventPayload$Installation.lateBind(GHEventPayload.java:295)

Haarolean avatar Aug 02 '23 17:08 Haarolean

This straightforward solution kinda fixes the issue, but until you try to call something like event.getRepositories().get(0).getOwner() which will result in something like this:

java.lang.NullPointerException: Cannot read field "login" because "this.owner" is null
	at org.kohsuke.github.GHRepository.getOwnerName(GHRepository.java:634)
	at org.kohsuke.github.GHRepository.getOwner(GHRepository.java:396)

Not sure how to deal with this. Should we clear the repo list in the payload completely to avoid any issues? Let me know what you think.

The solution:

Subject: [PATCH] Do not populate repo list in case of installation:deleted event
---
Index: src/main/java/org/kohsuke/github/GHEventPayload.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/org/kohsuke/github/GHEventPayload.java b/src/main/java/org/kohsuke/github/GHEventPayload.java
--- a/src/main/java/org/kohsuke/github/GHEventPayload.java	(revision 5380f59f380747deeb0553ca0a67ee27812218da)
+++ b/src/main/java/org/kohsuke/github/GHEventPayload.java	(revision fcd07d93436b1303b4f6133b059425252bcf4bec)
@@ -286,7 +286,7 @@
                         "Expected installation payload, but got something else. Maybe we've got another type of event?");
             }
             super.lateBind();
-            if (repositories != null && !repositories.isEmpty()) {
+            if (repositories != null && !repositories.isEmpty() && !"deleted".equalsIgnoreCase(getAction())) {
                 try {
                     for (GHRepository singleRepo : repositories) {
                         // populate each repository

Haarolean avatar Aug 02 '23 18:08 Haarolean