FeignHttpMessageConverters concurrency issue

[alianman]

Describe the bug
When mutiple concurrent threads call the org.springframework.cloud.openfeign.support.FeignHttpMessageConverters#getConverters method on a non-initialized class, some non-initialized converters could be returned. This leads to similar exceptions when processing requests/responses. After the class is initialized, no exceptions are thrown.

feign.codec.EncodeException: Could not write request: no suitable HttpMessageConverter found for request type [com.MyClass]
	at org.springframework.cloud.openfeign.support.SpringEncoder.encodeWithMessageConverter(SpringEncoder.java:168)
	at org.springframework.cloud.openfeign.support.SpringEncoder.encode(SpringEncoder.java:110)
	at org.springframework.cloud.openfeign.support.PageableSpringEncoder.encode(PageableSpringEncoder.java:105)
	at feign.RequestTemplateFactoryResolver$BuildEncodedTemplateFromArgs.resolve(RequestTemplateFactoryResolver.java:274)
	at feign.RequestTemplateFactoryResolver$BuildTemplateByResolvingArgs.create(RequestTemplateFactoryResolver.java:107)
	at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:48)
	at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:104)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory$1.proceed(FeignCachingInvocationHandlerFactory.java:66)
	at org.springframework.cache.interceptor.CacheInterceptor.lambda$invoke$0(CacheInterceptor.java:54)
	at org.springframework.cache.interceptor.CacheAspectSupport.invokeOperation(CacheAspectSupport.java:424)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:410)
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory.lambda$create$0(FeignCachingInvocationHandlerFactory.java:53)
	at jdk.proxy2/jdk.proxy2.$Proxy166.list(Unknown Source)
	... 189 filtered

java.lang.IllegalArgumentException: 'messageConverters' must not contain null elements
	at org.springframework.util.Assert.noNullElements(Assert.java:436)
	at org.springframework.web.client.HttpMessageConverterExtractor.<init>(HttpMessageConverterExtractor.java:78)
	at org.springframework.web.client.HttpMessageConverterExtractor.<init>(HttpMessageConverterExtractor.java:71)
	at org.springframework.cloud.openfeign.support.SpringDecoder.decode(SpringDecoder.java:56)
	at org.springframework.cloud.openfeign.support.ResponseEntityDecoder.decode(ResponseEntityDecoder.java:62)
	at feign.optionals.OptionalDecoder.decode(OptionalDecoder.java:38)
	at feign.InvocationContext.decode(InvocationContext.java:121)
	... 201 more
Wrapped by: <#64f634dc> feign.codec.DecodeException: 'messageConverters' must not contain null elements
	at feign.InvocationContext.decode(InvocationContext.java:125)
	at feign.InvocationContext.proceed(InvocationContext.java:94)
	at feign.ResponseHandler.handleResponse(ResponseHandler.java:69)
	at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:109)
	at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:53)
	at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:104)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory$1.proceed(FeignCachingInvocationHandlerFactory.java:66)
	at org.springframework.cache.interceptor.CacheInterceptor.lambda$invoke$0(CacheInterceptor.java:54)
	at org.springframework.cache.interceptor.CacheAspectSupport.invokeOperation(CacheAspectSupport.java:424)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:410)
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory.lambda$create$0(FeignCachingInvocationHandlerFactory.java:53)
	at jdk.proxy2/jdk.proxy2.$Proxy166.list(Unknown Source)
	... 172 filtered

Workaround

Current workaround is declaring own converters bean and call initialization explicitly at application startup:

    @Bean
    @ConditionalOnMissingBean
    public FeignHttpMessageConverters feignHttpMessageConverters(
            ObjectProvider<HttpMessageConverter<?>> messageConverters,
            ObjectProvider<HttpMessageConverterCustomizer> customizers) {
        var feignHttpMessageConverters = new FeignHttpMessageConverters(messageConverters, customizers);
        // init converters
        feignHttpMessageConverters.getConverters();
        return feignHttpMessageConverters;
    }

[fablefebvre-promod]

I had the same issue and the same workaround :)

[klach-ocado]

A note to above-mentioned workaround, current implementation of feignHttpMessageConverters was changed.

[westmc]

Any news on this? We are also running into this issue.

[DarkAlek]

Also have this issue with concurrency.

[sondemar]

Describe the bug When mutiple concurrent threads call the org.springframework.cloud.openfeign.support.FeignHttpMessageConverters#getConverters method on a non-initialized class, some non-initialized converters could be returned. This leads to similar exceptions when processing requests/responses. After the class is initialized, no exceptions are thrown.

feign.codec.EncodeException: Could not write request: no suitable HttpMessageConverter found for request type [com.MyClass]
	at org.springframework.cloud.openfeign.support.SpringEncoder.encodeWithMessageConverter(SpringEncoder.java:168)
	at org.springframework.cloud.openfeign.support.SpringEncoder.encode(SpringEncoder.java:110)
	at org.springframework.cloud.openfeign.support.PageableSpringEncoder.encode(PageableSpringEncoder.java:105)
	at feign.RequestTemplateFactoryResolver$BuildEncodedTemplateFromArgs.resolve(RequestTemplateFactoryResolver.java:274)
	at feign.RequestTemplateFactoryResolver$BuildTemplateByResolvingArgs.create(RequestTemplateFactoryResolver.java:107)
	at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:48)
	at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:104)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory$1.proceed(FeignCachingInvocationHandlerFactory.java:66)
	at org.springframework.cache.interceptor.CacheInterceptor.lambda$invoke$0(CacheInterceptor.java:54)
	at org.springframework.cache.interceptor.CacheAspectSupport.invokeOperation(CacheAspectSupport.java:424)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:410)
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory.lambda$create$0(FeignCachingInvocationHandlerFactory.java:53)
	at jdk.proxy2/jdk.proxy2.$Proxy166.list(Unknown Source)
	... 189 filtered

java.lang.IllegalArgumentException: 'messageConverters' must not contain null elements
	at org.springframework.util.Assert.noNullElements(Assert.java:436)
	at org.springframework.web.client.HttpMessageConverterExtractor.<init>(HttpMessageConverterExtractor.java:78)
	at org.springframework.web.client.HttpMessageConverterExtractor.<init>(HttpMessageConverterExtractor.java:71)
	at org.springframework.cloud.openfeign.support.SpringDecoder.decode(SpringDecoder.java:56)
	at org.springframework.cloud.openfeign.support.ResponseEntityDecoder.decode(ResponseEntityDecoder.java:62)
	at feign.optionals.OptionalDecoder.decode(OptionalDecoder.java:38)
	at feign.InvocationContext.decode(InvocationContext.java:121)
	... 201 more
Wrapped by: <#64f634dc> feign.codec.DecodeException: 'messageConverters' must not contain null elements
	at feign.InvocationContext.decode(InvocationContext.java:125)
	at feign.InvocationContext.proceed(InvocationContext.java:94)
	at feign.ResponseHandler.handleResponse(ResponseHandler.java:69)
	at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:109)
	at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:53)
	at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:104)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory$1.proceed(FeignCachingInvocationHandlerFactory.java:66)
	at org.springframework.cache.interceptor.CacheInterceptor.lambda$invoke$0(CacheInterceptor.java:54)
	at org.springframework.cache.interceptor.CacheAspectSupport.invokeOperation(CacheAspectSupport.java:424)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:410)
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory.lambda$create$0(FeignCachingInvocationHandlerFactory.java:53)
	at jdk.proxy2/jdk.proxy2.$Proxy166.list(Unknown Source)
	... 172 filtered

Workaround

Current workaround is declaring own converters bean and call initialization explicitly at application startup:

@Bean
@ConditionalOnMissingBean
public FeignHttpMessageConverters feignHttpMessageConverters(
        ObjectProvider<HttpMessageConverter<?>> messageConverters,
        ObjectProvider<HttpMessageConverterCustomizer> customizers) {
    var feignHttpMessageConverters = new FeignHttpMessageConverters(messageConverters, customizers);
    // init converters
    feignHttpMessageConverters.getConverters();
    return feignHttpMessageConverters;
}

One caveat about this workaround. Creating FeignHttpMessageConverters in the parent application context can change Feign behavior, because per-client converter customizers may no longer be applied. Keeping it in the Feign child context looks safer.

[klach-ocado]

One caveat about this workaround. Creating FeignHttpMessageConverters in the parent application context can change Feign behavior, because per-client converter customizers may no longer be applied. Keeping it in the Feign child context looks safer.

Maybe such an approach:

    @Bean
    static BeanPostProcessor feignHttpMessageConvertersInitializer() {
        return new FeignHttpMessageConvertersInitializer();
    }

    static class FeignHttpMessageConvertersInitializer implements BeanPostProcessor {

        @Override
        public Object postProcessAfterInitialization(Object bean, String beanName) {
            if (bean instanceof FeignClientFactory feignClientFactory) {
                Field configurationsField = ReflectionUtils.findField(FeignClientFactory.class, "configurations");
                ReflectionUtils.makeAccessible(configurationsField);
                Map<String, ?> configurations = (Map<String, ?>) ReflectionUtils.getField(configurationsField, feignClientFactory);
                configurations.keySet().forEach(name -> {
                    var convertersContainer = feignClientFactory.getInstance(name, FeignHttpMessageConverters.class);
                    convertersContainer.getConverters();
                });
            }
            return bean;
        }
    }

[sondemar]

One caveat about this workaround. Creating FeignHttpMessageConverters in the parent application context can change Feign behavior, because per-client converter customizers may no longer be applied. Keeping it in the Feign child context looks safer.

Maybe such an approach:

    @Bean
    static BeanPostProcessor feignHttpMessageConvertersInitializer() {
        return new FeignHttpMessageConvertersInitializer();
    }

    static class FeignHttpMessageConvertersInitializer implements BeanPostProcessor {

        @Override
        public Object postProcessAfterInitialization(Object bean, String beanName) {
            if (bean instanceof FeignClientFactory feignClientFactory) {
                Field configurationsField = ReflectionUtils.findField(FeignClientFactory.class, "configurations");
                ReflectionUtils.makeAccessible(configurationsField);
                Map<String, ?> configurations = (Map<String, ?>) ReflectionUtils.getField(configurationsField, feignClientFactory);
                configurations.keySet().forEach(name -> {
                    var convertersContainer = feignClientFactory.getInstance(name, FeignHttpMessageConverters.class);
                    convertersContainer.getConverters();
                });
            }
            return bean;
        }
    }

This looks closer 👍 I think the safer shape is still to keep FeignHttpMessageConverters child-scoped and only initialize it early from inside each Feign child context, instead of using a parent bean or a BeanPostProcessor with reflection?
How about:

  @Configuration(proxyBeanMethods = false)
  class FeignHttpMessageConvertersWorkaroundConfiguration {

      @Bean
      FeignClientSpecification feignHttpMessageConvertersWorkaroundSpecification() {
          return new FeignClientSpecification(
                  "default." + FeignHttpMessageConvertersWorkaroundConfiguration.class.getName(),
                  "default",
                  new Class<?>[] { FeignChildConfiguration.class });
      }

      @Configuration(proxyBeanMethods = false)
      static class FeignChildConfiguration {

          @Bean
          SmartInitializingSingleton feignHttpMessageConvertersInitializer(
                  ObjectProvider<FeignHttpMessageConverters> converters) {
              return () -> converters.ifAvailable(FeignHttpMessageConverters::getConverters);
          }
      }
  }

[klach-ocado]

The above FeignHttpMessageConvertersWorkaroundConfiguration looks more clean 👍, but I have the BeanPostProcessor already deployed, so I won't plan replacing one workaround with another.

[VolkerKruse]

There is already a PR for this issue. But unfortunately it got somehow stalled in the process:
#1372 (forwarded from original PR #1374)

[GJohannes]

Same concurrency issue on startup. Manually overwriting FeignHttpMessageConverters and using the implementation from #1372 works for now.

[Cal1br]

I encountered this issue too, while using multiple @aysnc workers.