feat(proxy): implement proxy configuration and detection for microbot#1455
feat(proxy): implement proxy configuration and detection for microbot#1455chsami merged 3 commits intodevelopmentfrom
Conversation
WalkthroughCentralizes SOCKS5 proxy setup and runtime proxy verification; redesigns the SplashScreen with rounded UI and periodic random-fact polling; improves Microbot version-title updates; refactors Microbot plugin list building; adds ProxyChecker, ProxyConfiguration, RandomFactClient, and minor logging/import adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant App as RuneLite
participant Splash as SplashScreen
participant ProxyCfg as ProxyConfiguration
participant HTTP as OkHttpClient
participant ProxyChk as ProxyChecker
participant UI as ClientUI
participant MB as Microbot
User->>App: Launch with options
App->>Splash: stage("Setting up proxy")
App->>ProxyCfg: setupProxy(options, proxyInfo)
App->>Splash: stage("Testing proxy address...")
App->>HTTP: init OkHttpClient
App->>ProxyChk: getDetectedIp(HTTP)
alt IP detected
App->>UI: set proxyMessage("Proxy enabled: <ip>")
UI-->>User: Title shows proxy info
else detection failed
App->>MB: showMessage("Failed to detect proxy")
App->>App: System.exit(1)
end
sequenceDiagram
autonumber
participant App
participant Splash as SplashScreen
participant Sched as ScheduledExecutor
participant Facts as RandomFactClient
participant UI as Splash UI
App->>Splash: init()
Splash->>Sched: scheduleAtFixedRate(fetchFact, 20s)
loop every 20s
Sched->>Facts: getRandomFactAsync(callback)
Facts-->>Splash: callback(fact) [on EDT]
Splash->>Splash: setFact(fact)
Splash->>UI: PropertyChange("fact")
UI-->>User: Updated fact displayed
end
App->>Splash: stop()
Splash->>Sched: cancel()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| * @param remoteVersion | ||
| * @param localVersion | ||
| */ | ||
| private void appendToTitle(String remoteVersion, String localVersion) { |
There was a problem hiding this comment.
This has already been resolved here: 943ba82
I had placed some shared methods I was using in the Microbot Plugin Manager into Rs2UiHelper
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotApi.java (1)
48-56: Handle HTTP errors and null bodies in microbotOpen().If the server returns a non-2xx or an empty body, this will either parse null or throw later. Add
isSuccessful()check and validatebody != null. Also prefer try-with-resources onResponseBodyto be explicit.Apply this diff:
- public UUID microbotOpen() throws IOException { - try (Response response = client.newCall(new Request.Builder().url(microbotApiUrl + "/session").build()).execute()) { - ResponseBody body = response.body(); - - InputStream in = body.byteStream(); - return gson.fromJson(new InputStreamReader(in, StandardCharsets.UTF_8), UUID.class); + public UUID microbotOpen() throws IOException { + try (Response response = client.newCall(new Request.Builder() + .url(microbotApiUrl + "/session") + .build()).execute()) { + if (!response.isSuccessful()) { + throw new IOException("Failed to open session: HTTP " + response.code()); + } + try (ResponseBody body = response.body()) { + if (body == null) { + throw new IOException("Empty response body"); + } + try (InputStream in = body.byteStream(); + InputStreamReader reader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + return gson.fromJson(reader, UUID.class); + } + } } catch (JsonParseException | IllegalArgumentException ex) // UUID.fromString can throw IllegalArgumentException { throw new IOException(ex); } }runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.java (1)
631-653: Loaded-plugin mapping likely broken (internalName vs class simple name).You key manifests by
internalNamebut key loaded plugins bygetClass().getSimpleName(). These rarely match (e.g., internalNameoneclickbankervs classOneClickBankerPlugin), sogroupwill be empty, hiding Configure and misreporting install state.Map both sides using a common normalized key derived from the plugin’s annotated name or another reliable identifier. Example fix:
- // Index loaded plugins by simple name (lowercased) - all instances for that name - Map<String, Collection<Plugin>> pluginsByName = loadedPlugins.stream() - .collect(Collectors.groupingBy( - p -> p.getClass().getSimpleName().toLowerCase(Locale.ROOT), - LinkedHashMap::new, - Collectors.toCollection(LinkedHashSet::new) - )); + // Normalize helper: lower-case, strip non-alphanumerics + Function<String, String> keyify = s -> s == null ? "" : s.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9]", ""); + + // Index loaded plugins by a normalized key from @PluginDescriptor.name (fallback to simple class name) + Map<String, Collection<Plugin>> pluginsByKey = loadedPlugins.stream() + .collect(Collectors.groupingBy(p -> { + PluginDescriptor d = p.getClass().getAnnotation(PluginDescriptor.class); + String name = (d != null ? d.name() : p.getClass().getSimpleName()); + return keyify.apply(name); + }, LinkedHashMap::new, Collectors.toCollection(LinkedHashSet::new))); @@ - plugins = manifestByName.entrySet().stream() + plugins = manifestByName.entrySet().stream() .map(e -> { String key = e.getKey(); MicrobotPluginManifest m = e.getValue(); - String simpleName = m.getInternalName(); - - Collection<Plugin> group = pluginsByName.getOrDefault(key, Collections.emptySet()); - int count = pluginCounts.getOrDefault(simpleName, -1); - boolean isInstalled = installed.stream().anyMatch(im -> im.getInternalName().equalsIgnoreCase(simpleName)); + String internal = m.getInternalName(); + Collection<Plugin> group = pluginsByKey.getOrDefault(key, Collections.emptySet()); + int count = pluginCounts.getOrDefault(internal, -1); + boolean isInstalled = installed.stream().anyMatch(im -> im.getInternalName().equalsIgnoreCase(internal)); return new PluginItem(m, group, count, isInstalled); }) .collect(Collectors.toList());This keeps sorting/counts intact and restores proper Configure wiring.
🧹 Nitpick comments (13)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotApi.java (2)
5-5: Remove unused Lombok logger or add logs.You added
@Slf4jbut there are no logging calls in this class. Either remove the annotation to avoid bytecode/field clutter or add meaningful logs (e.g., request failures).
95-102: Close response with try-with-resources and check status in microbotDelete().The call ignores HTTP status and manually closes the response. Use try-with-resources and validate success.
- public void microbotDelete(UUID uuid) throws IOException { - Request request = new Request.Builder() - .delete() - .url(microbotApiUrl + "/session?sessionId=" + uuid) - .build(); - - client.newCall(request).execute().close(); - } + public void microbotDelete(UUID uuid) throws IOException { + Request request = new Request.Builder() + .delete() + .url(microbotApiUrl + "/session?sessionId=" + uuid) + .build(); + try (Response response = client.newCall(request).execute()) { + if (!response.isSuccessful()) { + throw new IOException("Failed to delete session: HTTP " + response.code()); + } + } + }runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java (2)
169-170: Proxy message may be stale in window title.
this.titleis computed once using the current staticproxyMessage. IfproxyMessageis set after ClientUI construction (likely), the title won’t reflect it. EnsureClientUI.proxyMessageis set before constructing ClientUI or update the frame title after proxy detection.Minimal approach (call after setting
ClientUI.proxyMessagein RuneLite/RuneLiteDebug):if (ClientUI.getFrame() != null) { ClientUI.getFrame().setTitle(ClientUI.getFrame().getTitle() + ClientUI.proxyMessage); }If you prefer a robust solution, I can provide a small refactor to keep a
baseTitleand recompute the title wheneverproxyMessagechanges.
169-169: Avoid trailing space when no proxy message.Concatenate
proxyMessageonly when non-empty to avoid a stray space in the title.- this.title = title + (safeMode ? " (safe mode)" : " V" + version) + " " + proxyMessage; + this.title = title + (safeMode ? " (safe mode)" : " V" + version) + (proxyMessage.isEmpty() ? "" : " " + proxyMessage);runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java (2)
3-9: Deduplicate imports.There are repeated imports (
@Slf4j,ClientUI,RuneLiteProperties,Singleton,SwingUtilities). Clean these up to reduce noise.Also applies to: 18-24
155-175: Remove unusedisLocalVersionLower()after switching toRs2UiHelper.compareVersions.- private boolean isLocalVersionLower(String localVersion, String remoteVersion) { - String[] local = localVersion.split("\\."); - String[] remote = remoteVersion.split("\\."); - int length = Math.min(local.length, remote.length); - for (int i = 0; i < length; i++) { - int localPart = Integer.parseInt(local[i]); - int remotePart = Integer.parseInt(remote[i]); - if (localPart != remotePart) { - return localPart < remotePart; - } - } - return local.length < remote.length; - } + // removed; use Rs2UiHelper.compareVersions(local, remote) insteadrunelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.java (1)
91-99: Remove unusedhasIcon()or use it to skip queueing blank icons.
hasIcon()is unused. Either remove it or use it inpaint()to avoid queueing wheniconUrlis blank.- private boolean hasIcon() { - return !iconUrl.isBlank(); - } + private boolean hasIcon() { + return iconUrl != null && !iconUrl.isBlank(); + }And:
- if (!loadingStarted) { + if (!loadingStarted && hasIcon()) { loadingStarted = true; ... }runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java (4)
62-65: Remove unused escape() helper.
escape(String)is unused after switching to JTextArea.- static String escape(String s) { - return s.replace("&", "&").replace("<", "<").replace(">", ">"); - }
103-111: Splash close behavior: consider graceful shutdown instead of System.exit(0).The window’s close button is EXIT_ON_CLOSE and the custom “×” triggers
System.exit(0). This can abruptly terminate startup tasks.Consider calling
SplashScreen.stop()and letting the main app decide shutdown, or prompt the user.Also applies to: 72-101
281-286: Duplicate progress maximum assignment.
progress.setMaximum(1000)is set both in the constructor and inactionPerformed. Keep it in one place.- progress.setMaximum(1000);
22-25: Prefer single, shared ScheduledExecutorService (project convention).Per prior learnings, the project uses a shared single-thread scheduler. Consider reusing it (DI) instead of creating a new pool here.
Also applies to: 60-61
runelite-client/src/main/java/net/runelite/client/RuneLite.java (2)
190-191: CLI help: clarify encoding and SOCKS5H hint.Consider noting “URL‑encode special characters in user/pass” and mention socks5h for remote DNS (avoids DNS leaks).
- final ArgumentAcceptingOptionSpec<String> proxyInfo = parser.accepts("proxy", "Use a specified proxy. Format: scheme://user:pass@host:port") + final ArgumentAcceptingOptionSpec<String> proxyInfo = parser.accepts( + "proxy", + "Use a specified proxy. Format: scheme://user:pass@host:port (URL‑encode special characters). For SOCKS with remote DNS, use socks5h." + )Also mirror this description in RuneLiteDebug for consistency.
267-277: On proxy test failure, stop Splash and harden IP check.Minor UX and robustness: stop the splash before exiting and treat null/blank the same; also tighten the message.
- SplashScreen.stage(0, "Setting up proxy", "Testing proxy address..."); + SplashScreen.stage(0, "Setting up proxy", "Testing proxy address..."); if (options.has(proxyInfo)) { - String ip = ProxyChecker.getDetectedIp(okHttpClient); - if (ip.isEmpty()) { - Microbot.showMessage("Failed to detect external IP address, check your proxy settings. \n\n Make sure to use the format scheme://user:pass@host:port"); - System.exit(1); - } - - ClientUI.proxyMessage = " - Proxy enabled (detected IP " + ip + ")"; + String ip = ProxyChecker.getDetectedIp(okHttpClient); + if (ip == null || ip.isBlank()) { + Microbot.showMessage("Failed to detect external IP address. Check your proxy settings.\n\nExpected format: scheme://user:pass@host:port (URL‑encode special characters)."); + SplashScreen.stop(); + System.exit(1); + } + log.info("Proxy enabled; detected external IP {}", ip); + ClientUI.proxyMessage = " - Proxy enabled (detected IP " + ip + ")"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
runelite-client/src/main/java/net/runelite/client/RuneLite.java(4 hunks)runelite-client/src/main/java/net/runelite/client/RuneLiteDebug.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotApi.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.java(3 hunks)runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java(2 hunks)runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T16:17:37.615Z
Learnt from: g-mason0
PR: chsami/Microbot#1423
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/externalplugins/MicrobotPluginManager.java:366-389
Timestamp: 2025-08-23T16:17:37.615Z
Learning: The ScheduledExecutorService injected into MicrobotPluginManager (and other RuneLite classes) is configured as a single-threaded executor via Executors.newSingleThreadScheduledExecutor() at RuneLiteModule.java line 130, wrapped in ExecutorServiceExceptionLogger. This ensures all async tasks execute sequentially, preventing race conditions.
Applied to files:
runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotApi.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java (1)
Slf4j(25-200)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java (1)
runelite-client/src/main/java/net/runelite/client/RuneLiteProperties.java (1)
RuneLiteProperties(35-162)
🪛 GitHub Actions: CI
runelite-client/src/main/java/net/runelite/client/RuneLiteDebug.java
[error] 48-48: package net.runelite.client.proxy does not exist
runelite-client/src/main/java/net/runelite/client/RuneLite.java
[error] 48-48: package net.runelite.client.proxy does not exist
[error] 49-49: package net.runelite.client.proxy does not exist
runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java
[error] 4-4: Cannot find symbol: RandomFactClient in package net.runelite.client.plugins.microbot
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
runelite-client/src/main/java/net/runelite/client/RuneLite.java (1)
252-253: Good centralization of proxy setup.Calling ProxyConfiguration.setupProxy(...) early is the right place so the subsequent OkHttp client inherits settings.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/RandomFactClient.java (6)
30-39: Specify Accept and sanitize payload; avoid trailing whitespace.Add an Accept header and trim the body to keep UI clean.
- Request request = new Request.Builder() - .url(MICROBOT_API_URL + "/fact/random") - .build(); + Request request = new Request.Builder() + .url(MICROBOT_API_URL + "/fact/random") + .header("Accept", "text/plain") + .build(); ... - return response.body().string(); + return response.body().string().trim();
35-37: Unify error contract and add logging (avoid returning null).Currently non-200 returns a string message while exceptions return null. This inconsistency complicates the UI path and risks NPEs. Always return the same fallback string and log errors.
- if (!response.isSuccessful() || response.body() == null) { - return "Failed to fetch random fact"; - } + if (!response.isSuccessful() || response.body() == null) { + log.warn("Random fact endpoint failed with HTTP {}", response.code()); + return "Failed to fetch random fact"; + } ... - } catch (Exception e) { - return null; + } catch (Exception e) { + log.error("Error fetching random fact", e); + return "Failed to fetch random fact"; }Also applies to: 40-42
57-60: Guard callback from nulls for safer UI updates.Ensure the UI always receives a non-null string to render.
- SwingUtilities.invokeLater(() -> callback.accept(fact)); + SwingUtilities.invokeLater(() -> callback.accept( + fact != null ? fact : "Failed to fetch random fact" + ));
49-56: Prefer a small dedicated executor over the common pool.Using the common ForkJoinPool for blocking I/O can starve other tasks. A single-daemon thread is enough for a 20s poll.
Example (outside this range):
private static final java.util.concurrent.ExecutorService FACT_EXECUTOR = java.util.concurrent.Executors.newSingleThreadExecutor(r -> { Thread t = new Thread(r, "random-fact"); t.setDaemon(true); return t; }); // then: CompletableFuture.supplyAsync(RandomFactClient::getRandomFact, FACT_EXECUTOR)
16-18: Make utility class non-instantiable.Minor hygiene: mark final and add a private constructor.
-@Slf4j -public class RandomFactClient { +@Slf4j +public final class RandomFactClient { + private RandomFactClient() {}
24-27: Clarify return semantics in Javadoc.Document that the method never throws and returns a human-friendly fallback on failure. This helps callers avoid defensive null checks.
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java (1)
105-108: Remove unreachable fallback for port.After validation,
portis always > 0; simplify.- System.setProperty("socksProxyPort", String.valueOf(port > 0 ? port : 1080)); + System.setProperty("socksProxyPort", String.valueOf(port));runelite-client/src/main/java/net/runelite/client/proxy/ProxyChecker.java (2)
23-26: Add fallback IP endpoints for resiliency.Avoid single-point dependency on microbot.cloud by adding common IP echo services.
- List<String> endpoints = List.of( - "https://microbot.cloud/api/network/ip" - ); + List<String> endpoints = List.of( + "https://microbot.cloud/api/network/ip", + "https://api.ipify.org", + "https://ipv4.icanhazip.com" + );
14-16: Support IPv6 addresses as well.Some proxies/networks expose IPv6 egress; broaden validation (e.g., Guava InetAddresses).
Example change (requires Guava, commonly present in RuneLite):
import com.google.common.net.InetAddresses; // ... String ip = res.body().string().trim(); if (!InetAddresses.isInetAddress(ip)) { log.warn("Invalid IP payload from {}: '{}'", url, ip); return null; }Also applies to: 50-53
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/RandomFactClient.java(1 hunks)runelite-client/src/main/java/net/runelite/client/proxy/ProxyChecker.java(1 hunks)runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T19:20:28.774Z
Learnt from: chsami
PR: chsami/Microbot#1455
File: runelite-client/src/main/java/net/runelite/client/RuneLiteDebug.java:224-233
Timestamp: 2025-08-31T19:20:28.774Z
Learning: The RuneLite project only supports SOCKS5 proxies, and all traffic is expected to go through the proxy when configured.
Applied to files:
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/proxy/ProxyChecker.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/RandomFactClient.java (1)
Slf4j(16-65)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/RandomFactClient.java (1)
runelite-client/src/main/java/net/runelite/client/proxy/ProxyChecker.java (1)
Slf4j(12-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/RandomFactClient.java (1)
57-63: EDT handoff is correct.Good job ensuring UI updates happen on the Swing EDT.
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java (1)
64-69: SOCKS-only enforcement aligns with project policy.Rejecting HTTP(S) proxies matches the stated requirement that only SOCKS5 is supported and all traffic must go through the proxy.
runelite-client/src/main/java/net/runelite/client/proxy/ProxyChecker.java (1)
42-59: Networking flow looks solid.Good use of try-with-resources, status/body checks, trimming, and targeted logging.
| package net.runelite.client.plugins.microbot; | ||
|
|
||
| import lombok.extern.slf4j.Slf4j; | ||
| import okhttp3.OkHttpClient; | ||
| import okhttp3.Request; | ||
| import okhttp3.Response; | ||
|
|
||
| import javax.swing.*; | ||
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.function.Consumer; | ||
|
|
||
| /** | ||
| * Client to fetch random facts from the microbot API. | ||
| */ | ||
| @Slf4j | ||
| public class RandomFactClient { | ||
| private static final String MICROBOT_API_URL = "https://microbot.cloud/api"; | ||
| private static final OkHttpClient client = new OkHttpClient.Builder() | ||
| .connectTimeout(10, TimeUnit.SECONDS) | ||
| .readTimeout(10, TimeUnit.SECONDS) | ||
| .build(); | ||
|
|
||
| /** | ||
| * Fetches a random fact from the microbot API. | ||
| * @return random fact string or error message | ||
| */ | ||
| public static String getRandomFact() { | ||
| try { | ||
| Request request = new Request.Builder() | ||
| .url(MICROBOT_API_URL + "/fact/random") | ||
| .build(); | ||
|
|
||
| try (Response response = client.newCall(request).execute()) { | ||
| if (!response.isSuccessful() || response.body() == null) { | ||
| return "Failed to fetch random fact"; | ||
| } | ||
| return response.body().string(); | ||
| } | ||
| } catch (Exception e) { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Fetches a random fact asynchronously and invokes the callback on the Swing EDT. | ||
| * @param callback | ||
| */ | ||
| public static void getRandomFactAsync(Consumer<String> callback) { | ||
| CompletableFuture.supplyAsync(() -> { | ||
| try { | ||
| return getRandomFact(); | ||
| } catch (Exception e) { | ||
| log.error("Error in async fact fetching", e); | ||
| } | ||
| return null; | ||
| }).thenAccept(fact -> { | ||
| // Ensure UI update happens on EDT | ||
| SwingUtilities.invokeLater(() -> callback.accept(fact)); | ||
| }).exceptionally(throwable -> { | ||
| log.error("Async fact fetching failed", throwable); | ||
| return null; | ||
| }); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Quick cross-check: ensure we actually pass the proxy-configured client into RandomFactClient.
If you adopt setClient, verify at startup that it’s invoked with the same OkHttpClient used by ProxyChecker/proxy config.
🏁 Script executed:
#!/bin/bash
# Where is OkHttpClient built and how is RandomFactClient used?
rg -nP --type=java -C3 'new OkHttpClient\.Builder\('
rg -nP --type=java -C3 '\bRandomFactClient\.(getRandomFact|getRandomFactAsync|setClient)\b'Length of output: 3525
Inject proxy-configured OkHttpClient into RandomFactClient
Add a public static void setClient(OkHttpClient client) to RandomFactClient (in runelite-client/src/main/java/net/runelite/client/plugins/microbot/RandomFactClient.java) and invoke it at startup—passing the same OkHttpClient built by RuneLite.buildHttpClient(…)—so that all requests use the proxy settings.
There was a problem hiding this comment.
We use SOCKS5 for proxies. all of the traffic will pass through it
There was a problem hiding this comment.
@chsami, thank you for the clarification! That makes perfect sense. Since SOCKS5 proxy configuration is applied at the JVM level and routes all traffic through the proxy automatically, the RandomFactClient will inherently use the proxy settings without needing explicit client injection. This is a much cleaner approach than what I initially suggested.
I appreciate you explaining how the proxy system works in your implementation.
🐰
👂 <- learned something new!
/||
/\
✏️ Learnings added
Learnt from: chsami
PR: chsami/Microbot#1455
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/RandomFactClient.java:1-65
Timestamp: 2025-08-31T19:37:01.637Z
Learning: In the Microbot/RuneLite project, SOCKS5 proxy configuration is applied at the JVM level, meaning all network traffic (including HTTP clients like RandomFactClient) automatically routes through the configured proxy without requiring explicit client configuration injection.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: chsami
PR: chsami/Microbot#1455
File: runelite-client/src/main/java/net/runelite/client/RuneLiteDebug.java:224-233
Timestamp: 2025-08-31T19:20:28.793Z
Learning: The RuneLite project only supports SOCKS5 proxies, and all traffic is expected to go through the proxy when configured.
| private static final OkHttpClient client = new OkHttpClient.Builder() | ||
| .connectTimeout(10, TimeUnit.SECONDS) | ||
| .readTimeout(10, TimeUnit.SECONDS) | ||
| .build(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Route HTTP through the shared, proxy-configured OkHttpClient (don’t bypass proxy).
This private static client ignores the proxy stack introduced in this PR, so fact fetches may fail behind SOCKS5 or skip configured auth. Allow injecting the proxy-configured client (or reuse the app-wide one) and add a total call timeout.
Apply:
- private static final OkHttpClient client = new OkHttpClient.Builder()
+ private static volatile OkHttpClient client = new OkHttpClient.Builder()
.connectTimeout(10, TimeUnit.SECONDS)
.readTimeout(10, TimeUnit.SECONDS)
+ .callTimeout(10, TimeUnit.SECONDS)
.build();
+
+ /** Optionally set by ProxyConfiguration to reuse the proxy-configured/shared client. */
+ public static void setClient(OkHttpClient okHttpClient) {
+ if (okHttpClient != null) {
+ client = okHttpClient;
+ }
+ }Follow-up: from your proxy/bootstrap path, call RandomFactClient.setClient(proxyConfiguredOkHttpClient). I can wire this where you initialize the proxy if you’d like.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static final OkHttpClient client = new OkHttpClient.Builder() | |
| .connectTimeout(10, TimeUnit.SECONDS) | |
| .readTimeout(10, TimeUnit.SECONDS) | |
| .build(); | |
| private static volatile OkHttpClient client = new OkHttpClient.Builder() | |
| .connectTimeout(10, TimeUnit.SECONDS) | |
| .readTimeout(10, TimeUnit.SECONDS) | |
| .callTimeout(10, TimeUnit.SECONDS) | |
| .build(); | |
| /** Optionally set by ProxyConfiguration to reuse the proxy-configured/shared client. */ | |
| public static void setClient(OkHttpClient okHttpClient) { | |
| if (okHttpClient != null) { | |
| client = okHttpClient; | |
| } | |
| } |
| public static String getDetectedIp(OkHttpClient okHttpClient) { | ||
| List<String> endpoints = List.of( | ||
| "https://microbot.cloud/api/network/ip" | ||
| ); | ||
|
|
||
| for (String url : endpoints) { | ||
| String ip = fetchIp(okHttpClient, url); | ||
| if (ip != null) { | ||
| return ip; | ||
| } | ||
| } | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure client timeouts are set at call sites.
This class relies on the provided OkHttpClient; verify connect/read/write timeouts are non-default and sane (e.g., 5–15s) to avoid hangs when proxies misbehave.
Run to locate builders and timeout settings and the call sites:
Also applies to: 42-59
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP -C2 --type=java 'new\s+OkHttpClient\.Builder\s*\('
rg -nP --type=java 'connectTimeout\s*\('
rg -nP --type=java 'readTimeout\s*\('
rg -nP --type=java 'writeTimeout\s*\('
rg -nP -C3 --type=java 'ProxyChecker\.getDetectedIp\s*\('Length of output: 2385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nF 'getDetectedIp' -n runelite-client/srcLength of output: 474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP -C10 --type=java 'static OkHttpClient buildHttpClient' runelite-client/src/main/java/net/runelite/client/RuneLiteDebug.java
rg -nP -C10 --type=java 'static OkHttpClient buildHttpClient' runelite-client/src/main/java/net/runelite/client/RuneLite.javaLength of output: 1526
Explicitly set connect, read, and write timeouts on OkHttpClient builders
- In runelite-client/src/main/java/net/runelite/client/RuneLiteDebug.java (buildHttpClient): only pingInterval and interceptor are configured—add .connectTimeout(...), .readTimeout(...), .writeTimeout(...) (e.g. 10s).
- In runelite-client/src/main/java/net/runelite/client/RuneLite.java (buildHttpClient): likewise add explicit timeouts to avoid relying solely on OkHttp defaults.
🤖 Prompt for AI Agents
In runelite-client/src/main/java/net/runelite/client/RuneLiteDebug.java around
the buildHttpClient method and in
runelite-client/src/main/java/net/runelite/client/RuneLite.java around its
buildHttpClient method (both referenced in the review), the OkHttpClient
builders only set pingInterval and interceptors and rely on OkHttp defaults for
timeouts; update each OkHttpClient.Builder to explicitly set connectTimeout,
readTimeout, and writeTimeout (for example to 10 seconds) so the client doesn't
depend on defaults—add the three timeout calls to the builder chain alongside
the existing settings.
| URI uri = URI.create(options.valueOf(proxyInfo)); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle invalid proxy URIs gracefully (avoid crash on bad input).
URI.create(...) throws IllegalArgumentException on malformed values; show a friendly message and exit instead of bubbling an uncaught exception.
- URI uri = URI.create(options.valueOf(proxyInfo));
+ URI uri;
+ try {
+ uri = URI.create(String.valueOf(options.valueOf(proxyInfo)));
+ } catch (IllegalArgumentException e) {
+ Microbot.showMessage("Invalid proxy URI. Expected format: socks5://[user[:pass]@]host:port");
+ System.exit(1);
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| URI uri = URI.create(options.valueOf(proxyInfo)); | |
| URI uri; | |
| try { | |
| uri = URI.create(String.valueOf(options.valueOf(proxyInfo))); | |
| } catch (IllegalArgumentException e) { | |
| Microbot.showMessage("Invalid proxy URI. Expected format: socks5://[user[:pass]@]host:port"); | |
| System.exit(1); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java
around lines 28-29, replace the direct call to
URI.create(options.valueOf(proxyInfo)) with code that wraps URI creation in a
try/catch for IllegalArgumentException (and optionally NullPointerException),
and on catch log or print a clear, user-friendly error indicating the proxy URI
is malformed (include the bad value), then exit with a non-zero status instead
of allowing the uncaught exception to propagate; ensure normal flow continues
when the URI is valid.
| if (options.has("proxy-type")) { | ||
| Microbot.showMessage("Proxy type is no longer supported, please use the format -proxy=socks://user:pass@host:port or http://user:pass@host:port"); | ||
| System.exit(1); | ||
| } |
There was a problem hiding this comment.
Contradictory user guidance: UI suggests HTTP supported while code rejects HTTP(S).
The message at Line 31 advertises http:// as valid, but validateProxyScheme disallows HTTP/HTTPS. Fix the message to avoid user confusion.
- Microbot.showMessage("Proxy type is no longer supported, please use the format -proxy=socks://user:pass@host:port or http://user:pass@host:port");
+ Microbot.showMessage("Proxy type flag is no longer supported. Use -proxy=socks5://[user[:pass]@]host:port (HTTP/HTTPS proxies are not supported).");Also applies to: 58-69
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java
around lines 30-33 and 58-69, the user-facing message says http:// is supported
while validateProxyScheme rejects HTTP/HTTPS; update the messages to reflect the
actual supported proxy schemes (e.g., only socks:// and socks5:// with optional
user:pass@host:port) or otherwise align validateProxyScheme to accept
http/https—modify the strings to remove any reference to http:// or https:// (or
change validation to permit them) so the UI guidance matches the code behavior.
| String host = uri.getHost(); | ||
| String scheme = Optional.ofNullable(uri.getScheme()).orElse("").toLowerCase(Locale.ROOT); | ||
|
|
||
| validateProxyScheme(scheme); | ||
|
|
||
| int port = validatePort(uri.getPort()); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate proxy host presence.
uri.getHost() can be null/blank; add a guard to prevent setting empty system properties.
- String host = uri.getHost();
+ String host = uri.getHost();
String scheme = Optional.ofNullable(uri.getScheme()).orElse("").toLowerCase(Locale.ROOT);
validateProxyScheme(scheme);
+ if (host == null || host.isBlank()) {
+ Microbot.showMessage("Invalid proxy host");
+ System.exit(1);
+ }
int port = validatePort(uri.getPort());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String host = uri.getHost(); | |
| String scheme = Optional.ofNullable(uri.getScheme()).orElse("").toLowerCase(Locale.ROOT); | |
| validateProxyScheme(scheme); | |
| int port = validatePort(uri.getPort()); | |
| String host = uri.getHost(); | |
| String scheme = Optional.ofNullable(uri.getScheme()).orElse("").toLowerCase(Locale.ROOT); | |
| validateProxyScheme(scheme); | |
| if (host == null || host.isBlank()) { | |
| Microbot.showMessage("Invalid proxy host"); | |
| System.exit(1); | |
| } | |
| int port = validatePort(uri.getPort()); |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java
around lines 35 to 41, uri.getHost() can be null or blank so add a guard to
validate host presence before using it or setting system properties: after
retrieving host, check if host is null or host.trim().isEmpty() and if so throw
an IllegalArgumentException (or return/skip proxy setup) with a clear message;
only proceed to call validateProxyScheme/validatePort and set system properties
when the host passes this non-empty check.
| configureProxy(host, port); | ||
|
|
||
| if (user != null) { | ||
| setupAuthenticator(user, pass); | ||
| } |
There was a problem hiding this comment.
Limit Authenticator to proxy requests and target host/port (avoid credential leakage).
Current Authenticator returns credentials for any request type/host; restrict to RequestorType.PROXY and the configured proxy host/port.
- if (user != null) {
- setupAuthenticator(user, pass);
- }
+ if (user != null) {
+ setupAuthenticator(user, pass, host, port);
+ }
@@
- private static void setupAuthenticator(String user, String pass) {
+ private static void setupAuthenticator(String user, String pass, String host, int port) {
Authenticator.setDefault(new Authenticator() {
@Override
protected PasswordAuthentication getPasswordAuthentication() {
- return new PasswordAuthentication(user, pass != null ? pass.toCharArray() : new char[0]);
+ if (getRequestorType() == RequestorType.PROXY
+ && host.equalsIgnoreCase(getRequestingHost())
+ && getRequestingPort() == port) {
+ return new PasswordAuthentication(user, pass != null ? pass.toCharArray() : new char[0]);
+ }
+ return null;
}
});
}Also applies to: 115-122
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java
around lines 46-50 and 115-122, the Authenticator currently returns credentials
for any request; restrict it to only respond to proxy authentication for the
configured proxy host and port. Update the
Authenticator.getPasswordAuthentication() to check that getRequestorType() ==
RequestorType.PROXY and that getRequestingHost()/getRequestingPort() match the
configured proxy host and port (and return null otherwise), and ensure the
configured host/port are accessible to the Authenticator (use final fields or
capture them in the closure) before returning a new PasswordAuthentication(user,
pass.toCharArray()) when and only when those checks pass.
| private static int validatePort(int port) { | ||
| if (port <= 0) { | ||
| Microbot.showMessage("Invalid proxy port"); | ||
| System.exit(1); | ||
| } | ||
| return port; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce valid TCP port range (1–65535).
Protect against out-of-range ports.
- private static int validatePort(int port) {
- if (port <= 0) {
- Microbot.showMessage("Invalid proxy port");
- System.exit(1);
- }
- return port;
- }
+ private static int validatePort(int port) {
+ if (port <= 0 || port > 65535) {
+ Microbot.showMessage("Invalid proxy port (must be 1-65535)");
+ System.exit(1);
+ }
+ return port;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static int validatePort(int port) { | |
| if (port <= 0) { | |
| Microbot.showMessage("Invalid proxy port"); | |
| System.exit(1); | |
| } | |
| return port; | |
| } | |
| private static int validatePort(int port) { | |
| if (port <= 0 || port > 65535) { | |
| Microbot.showMessage("Invalid proxy port (must be 1-65535)"); | |
| System.exit(1); | |
| } | |
| return port; | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/proxy/ProxyConfiguration.java
around lines 76 to 82, the validatePort method currently only checks port > 0;
update it to enforce the full valid TCP port range (1–65535) by checking that
port >= 1 && port <= 65535, and if the value is outside that range call
Microbot.showMessage with a clear error including the invalid port value and
then terminate (System.exit(1)) or otherwise abort initialization; return the
port when valid.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java (2)
4-4: Build breaker: hard import to missing RandomFactClient.This import fails CI if RandomFactClient isn’t present. Make it optional (reflection) and drop the import.
Apply alongside the scheduling fixes:
- import net.runelite.client.plugins.microbot.RandomFactClient;
61-63: Thread leak + shadowing + lack of optionality for RandomFactClient.
- Local var shadows the static executor, so stop() can’t shut it down.
- The pool uses non-daemon threads.
- Missing class or runtime errors will kill the scheduled task.
Apply this diff (pairs with removing the import at Line 4):
@@ - private static ScheduledExecutorService scheduledRandomFactExecutorService; - private static ScheduledFuture<?> scheduledRandomFactFuture; + private static ScheduledExecutorService scheduledRandomFactExecutor; + private static ScheduledFuture<?> scheduledRandomFactFuture; @@ - ScheduledExecutorService scheduledRandomFactExecutorService = Executors.newSingleThreadScheduledExecutor(); - scheduledRandomFactFuture = scheduledRandomFactExecutorService.scheduleAtFixedRate( - () -> { - RandomFactClient.getRandomFactAsync(SplashScreen::setFact); - }, - 0, 20, TimeUnit.SECONDS); + scheduledRandomFactExecutor = Executors.newSingleThreadScheduledExecutor(r -> { + Thread t = new Thread(r, "SplashScreenFacts"); + t.setDaemon(true); + return t; + }); + scheduledRandomFactFuture = scheduledRandomFactExecutor.scheduleAtFixedRate(() -> { + try { + Class<?> cls = Class.forName("net.runelite.client.plugins.microbot.RandomFactClient"); + var m = cls.getMethod("getRandomFactAsync", java.util.function.Consumer.class); + m.invoke(null, (java.util.function.Consumer<String>) SplashScreen::setFact); + } catch (Throwable t) { + log.debug("RandomFactClient unavailable or failed, skipping", t); + } + }, 0, 20, TimeUnit.SECONDS); @@ - if (scheduledRandomFactFuture != null) { - scheduledRandomFactFuture.cancel(true); - } - if (scheduledRandomFactExecutorService != null) { - scheduledRandomFactExecutorService.shutdownNow(); - scheduledRandomFactExecutorService = null; - } + if (scheduledRandomFactFuture != null) { + scheduledRandomFactFuture.cancel(true); + scheduledRandomFactFuture = null; + } + if (scheduledRandomFactExecutor != null) { + scheduledRandomFactExecutor.shutdownNow(); + scheduledRandomFactExecutor = null; + }Also applies to: 271-277, 323-330
🧹 Nitpick comments (4)
runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java (4)
95-99: Close button should clean up before exiting.Call stop() first to cancel the scheduled task/executor.
- public void mouseClicked(MouseEvent e) { - // Stop the application - System.exit(0); - } + public void mouseClicked(MouseEvent e) { + // Graceful shutdown + SplashScreen.stop(); + System.exit(0); + }
53-59: Let facts update even if the splash isn’t instantiated yet.Fire the event only when INSTANCE exists; always update the value.
- public static void setFact(String newFact) { - if (INSTANCE != null && newFact != null && !String.valueOf(factValue).equals(newFact)) { - String oldValue = factValue; - factValue = newFact; - INSTANCE.pcs.firePropertyChange("fact", oldValue, newFact); - } - } + public static void setFact(String newFact) { + if (newFact == null || String.valueOf(factValue).equals(newFact)) return; + String oldValue = factValue; + factValue = newFact; + if (INSTANCE != null) { + INSTANCE.pcs.firePropertyChange("fact", oldValue, newFact); + } + }
64-67: Unused helper.escape(...) is unused after switching to JTextArea.
- static String escape(String s) { - return s.replace("&", "&").replace("<", "<").replace(">", ">"); - } + // (removed) unused
271-277: Option: reuse the app’s single-threaded scheduler to avoid extra threads.If accessible at this layer, prefer the central ScheduledExecutorService configured in RuneLiteModule to keep all async tasks serialized and avoid thread proliferation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T16:17:37.615Z
Learnt from: g-mason0
PR: chsami/Microbot#1423
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/externalplugins/MicrobotPluginManager.java:366-389
Timestamp: 2025-08-23T16:17:37.615Z
Learning: The ScheduledExecutorService injected into MicrobotPluginManager (and other RuneLite classes) is configured as a single-threaded executor via Executors.newSingleThreadScheduledExecutor() at RuneLiteModule.java line 130, wrapped in ExecutorServiceExceptionLogger. This ensures all async tasks execute sequentially, preventing race conditions.
Applied to files:
runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java
🔇 Additional comments (1)
runelite-client/src/main/java/net/runelite/client/ui/SplashScreen.java (1)
341-353: Guard against divide-by-zero in progress math.If total can be 0, this will throw and also show “/ 0”.
- stage(startProgress + ((endProgress - startProgress) * done / total), actionText, subActionText, progress); + int denom = Math.max(total, 1); + stage(startProgress + ((endProgress - startProgress) * (double) done / denom), actionText, subActionText, progress);If total is legitimately 0, consider rendering “done / ?” instead.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Style