Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| def _run_health_probe(self, stats: ProxyStats) -> None: | ||
| target = self._health_target | ||
| host = target["host"] # type: ignore[index] | ||
| port = target["port"] # type: ignore[index] | ||
| path = target["path"] # type: ignore[index] | ||
| scheme = target["scheme"] # type: ignore[index] | ||
|
|
||
| overall_start = time.perf_counter() | ||
| payload: Dict[str, object] = {} | ||
| latency_ms: Optional[float] = None | ||
| ok = False | ||
| error: Optional[str] = None | ||
|
|
||
| stream: Optional[socket.socket] = None | ||
| sock: Optional[socket.socket] = None | ||
| response: Optional[http.client.HTTPResponse] = None | ||
| try: | ||
| sock, _ = self._connect_via_proxy( | ||
| stats, | ||
| host, | ||
| int(port), | ||
| self._health_timeout, | ||
| record_metrics=False, | ||
| ) | ||
| except ProxyTargetError as exc: | ||
| error = str(exc) | ||
| except ProxyHandshakeError as exc: | ||
| error = str(exc) | ||
| else: | ||
| stream = sock | ||
| try: | ||
| stream.settimeout(self._health_timeout) | ||
| except OSError: | ||
| pass | ||
| if scheme == "https": | ||
| try: | ||
| context = ssl.create_default_context() | ||
| stream = context.wrap_socket(stream, server_hostname=host) | ||
| except Exception as exc: | ||
| error = str(exc) | ||
| self._record_proxy_failure(stats, "health", error) | ||
| try: | ||
| sock.close() | ||
| except Exception: | ||
| pass | ||
| stream = None | ||
| if stream is not None: | ||
| try: | ||
| request_bytes = ( | ||
| f"GET {path or '/'} HTTP/1.1\r\n" | ||
| f"Host: {host}\r\n" | ||
| "User-Agent: mcsmartscan/1.0\r\n" | ||
| "Accept: application/json\r\n" | ||
| "Connection: close\r\n\r\n" | ||
| ).encode("ascii", "ignore") | ||
| stream.sendall(request_bytes) | ||
| response = http.client.HTTPResponse(stream) | ||
| response.begin() | ||
| body = response.read() | ||
| status_code = response.status | ||
| if status_code != 200: | ||
| raise RuntimeError(f"Health endpoint HTTP {status_code}") | ||
| try: | ||
| data = json.loads(body.decode("utf-8", "replace")) | ||
| except json.JSONDecodeError as exc: | ||
| raise RuntimeError(f"Invalid JSON: {exc}") from exc | ||
| if isinstance(data, dict): | ||
| payload = data # type: ignore[assignment] | ||
| else: | ||
| payload = {} | ||
| latency_ms = (time.perf_counter() - overall_start) * 1000.0 | ||
| ok = bool(payload.get("mullvad_exit_ip")) | ||
| if ok: | ||
| self._record_success(stats, latency_ms) | ||
| else: | ||
| error = "Mullvad exit flag false" |
There was a problem hiding this comment.
The new health checker calls _record_success(stats, latency_ms) whenever the Mullvad health endpoint returns mullvad_exit_ip (see _run_health_probe). _record_success increments stats.successes and clears consecutive_proxy_failures, which are the counters used by _apply_failure_penalty to quarantine or hard-disable an endpoint after too many failures. As a result, a proxy that repeatedly fails real scan connections but can still reach the Mullvad health URL will have its failure counters continually reset and successes incremented by background health probes, so it will never cross the FAIL_DISABLE_THRESHOLD. In previous behaviour only actual scan successes updated these counters. Consider keeping health metrics separate (e.g. do not call _record_success from health probes) so broken proxies can still be quarantined or disabled.
Useful? React with 👍 / 👎.
2 participants
No description provided.