slackapi / slackapi/java-slack-sdk

Flaky: AuthProxyHeadersTest.rtm intermittently sees 3 proxy tunnels, expects 2

Open
#1,630 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
602
Forks
232
Avg merge
4d 9h
Merged PRs (30d)
7

Description

Summary

test_with_remote_apis.AuthProxyHeadersTest.rtm is flaky. It asserts the embedded proxy handled exactly 2 connections:

assertThat(callCount.get(), is(2));

but intermittently observes 3 (Expected: is <2> ... but: was <3>), failing the build. The test's callCount is incremented by the embedded Jetty ConnectHandler on every non-407 CONNECT tunnel, so a 3 means an extra tunnel was opened through the proxy during the RTM flow.

Status of the attempted fix (this is the point of the issue)

A change on the api branch switched the call to rtmConnect(rtmBotToken, false) — dropping the fullUserInfoRequired users.info call on the theory that it was the extra HTTPS round-trip inflating the tunnel count to 3. See the change on api:

// two connections (rtm.connect + websocket), otherwise a third tunnel can be opened
try (RTMClient rtm = slack.rtmConnect(rtmBotToken, false)) {

That change does not make the test deterministic. A build that includes the false change on api still failed rtm with was <3> at the is(2) assertion. So dropping users.info was, at most, a partial mitigation — the extra CONNECT tunnel has another source (a WebSocket reconnect, a keep-alive revalidation opening a fresh tunnel, or similar timing). Whether the change lowers the flake rate is unconfirmed; it does not eliminate the flake.

Repro / evidence

Intermittent — the same test passes on other builds, and fails on others with the false change present. Confirmed by the fact that the assertion still hits was <3> on api, which carries the change.

Possible directions (not yet chosen)

  1. Assert a bounded range instead of an exact count — the test's real intent is "the proxy was used with the auth header," not "exactly N tunnels." A tolerant assertion (e.g. >= 2, or anyOf(is(2), is(3))) would kill the flake without weakening what's verified (a 407 or a leaked connection would still show up as a wrong count). Follows the same pattern already used in the sibling scim() test in this file (assertTrue(callCount.get() >= 1)).
  2. Find and eliminate the extra tunnel — trace whether the RTM WebSocket connect opens a second CONNECT under the proxy (reconnect / pooled-connection revalidation) and make the count deterministic. Harder; needs the proxy's per-CONNECT logging enabled to see the real tunnel sequence.

Filing so the flake is on the record and the false change on api isn't mistaken for a settled fix.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in slack-api-client/src/test/java/test_with_remote_apis/AuthProxyHeadersTest.java, focusing on rtm(), the embedded Jetty ConnectHandler count, and the sibling scim() assertion. Review the existing rtmConnect(..., false) change and trace the intermittent extra CONNECT tunnel. Done means the test is deterministic while still verifying proxy authentication behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
networking, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.