Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feign.Builder have a requestInterceptor bug #2249

Open
hdmvp opened this issue Nov 28, 2023 · 2 comments
Open

Feign.Builder have a requestInterceptor bug #2249

hdmvp opened this issue Nov 28, 2023 · 2 comments

Comments

@hdmvp
Copy link

hdmvp commented Nov 28, 2023

for example:
public static T getHttpApi(Class api, String url) {
ApplicationContext applicationContext = SpringContext.getApplicationContext();
return applicationContext.getBean(Feign.Builder.class).requestInterceptor(new CustomFeignInterceptor()).target(api, url);
}
add the requestInterceptor at here cause :

java.util.ConcurrentModificationException

at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1631)....

suggest:

 /**
     * Adds a single request interceptor to the builder.
     */
    public Builder requestInterceptor(RequestInterceptor requestInterceptor) {
      this.requestInterceptors.add(requestInterceptor);
      return this;
    }
 private final List<RequestInterceptor> requestInterceptors =
        new ArrayList<RequestInterceptor>();

the soureCode requestInterceptors should be a CopyOnWriteArrayList

@lquterqtd
Copy link

requestInterceptor只需要add一次就可以吧? 你这里貌似是并发添加了,可以控制一下调用的地方,只add一次

@hdmvp
Copy link
Author

hdmvp commented Dec 8, 2023

requestInterceptor只需要add一次就可以吧? 你这里貌似是并发添加了,可以控制一下调用的地方,只add一次

这里每次的调用确实会存在并发添加的问题,但是基于暴露的方法来看,如果不点进源码去看,根本就不会发现这个问题。所以基于一个良好的体验来讲,我觉得这个requestInterceptor方法的内部集合实现有改进的空间,换成 CopyOnWriteArraySet会更友好一点

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants