thoughtworks / metrik

An easy-to-use, cross-platform measurement tool that pulls data out of CD pipelines and analysis the four key metrics for you.
MIT License
354 stars 87 forks source link

Synchronize failed while fetching commits of deleted branch in Github #114

Closed prabhu43 closed 2 years ago

prabhu43 commented 2 years ago

Describe the bug Synchronize failed while fetching commits of deleted branch in Github

To Reproduce Steps to reproduce the behavior:

  1. Add a Github Actions project in which a branch is deleted after merging its commits
  2. See error

Expected behavior Synchronize should work and able to see metrics

Desktop (please complete the following information):

Error thrown ERROR 2022-05-15 09:36:11.299 traceId: --- [pool-2-thread-1] m.p.r.a.SynchronizationApplicationService:Synchronize failed for pipeline [627ff5a82a30707ac0e4ed11 - ordermanagement], error: [feign.codec.DecodeException: Error while extracting response for type [java.util.List] and content type [application/json;charset=utf-8]; nested exception is org.springframework.http.converter.HttpMessageNotReadableException: JSON parse error: Cannot deserialize instance of java.util.ArrayList<metrik.project.infrastructure.github.feign.response.CommitResponse> out of START_OBJECT token; nested exception is com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize instance of java.util.ArrayList<metrik.project.infrastructure.github.feign.response.CommitResponse> out of START_OBJECT token at [Source: (PushbackInputStream); line: 1, column: 1]] Exception in thread "pool-2-thread-1" metrik.project.exception.SynchronizationException: Synchronize failed at metrik.project.rest.applicationservice.SynchronizationApplicationService$synchronize$1.accept(SynchronizationApplicationService.kt:39) at metrik.project.rest.applicationservice.SynchronizationApplicationService$synchronize$1.accept(SynchronizationApplicationService.kt:14) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) at java.base/java.util.stream.ForEachOps$ForEachTask.compute(ForEachOps.java:290) at java.base/java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:746) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinTask.doInvoke(ForkJoinTask.java:408) at java.base/java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:736) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateParallel(ForEachOps.java:159) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel(ForEachOps.java:173) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:661) at metrik.project.rest.applicationservice.SynchronizationApplicationService.synchronize(SynchronizationApplicationService.kt:34) at metrik.project.rest.applicationservice.SynchronizationApplicationService$$FastClassBySpringCGLIB$$e40c75eb.invoke() at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218) at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:687) at metrik.project.rest.applicationservice.SynchronizationApplicationService$$EnhancerBySpringCGLIB$$ff0f05fc.synchronize() at metrik.project.rest.SynchronizationController$sseSynchronization$1.run(SynchronizationController.kt:53) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:834)

prabhu43 commented 2 years ago

What are we fetching commits of deleted branches here ? Shall we skip this process for deleted branches?

LixingSun commented 2 years ago

My team is having the exact same issue.

To better describe the problem here, want to clarify the current flow: when GithubActionsPipelineService.kt try to execute syncBuildsProgressively, it will first roughly fetch all runs that need to sync. However runs API only contains head commit instead of relevant commits brought by each run. So based on the design, after getting clear about initial runs, we need second step to make clear what are the commits for each run.

Issue occurred at this step that when a branch was deleted. Then run records for the deleted branch are still retrievable, while attempt to get commits will get 404 error in response due to the non-existing branch.

Current fix going to work on:

Will soon raise a PR for this.

kevinlzw commented 2 years ago

Hi @LixingSun, I think your solution should work.

LixingSun commented 2 years ago

The changes have been merged.