MOBIN-F commented on PR #12465:
URL: https://github.com/apache/seatunnel/pull/12465#issuecomment-5847593970
Thank you for the careful review. I have optimized the two issues.
**1. URL in logs (Issue 1)** — `[Improve][Connector-V2] Mask the WebSocket
url query parameter values in logs and errors`
`WebSocketSourceConfig` derives a `maskedUrl` once: userinfo dropped, every
query parameter
keeps its name but loses its value (`?streams=***&token=***`). I masked
values rather than
using `url.split("\\?")[0]` so a log can still tell one stream apart from
another, and masked
by position rather than by a deny list of parameter names — these APIs name
credential params
in too many ways for a list to be trustworthy.
Two things worth flagging:
- The leak was slightly wider than the listed sites: also
`WebSocketSourceReader` (the
DESERIALIZE_FAILED message and the two bounded-stop INFO lines) and the
scheme-validation
message in `WebSocketSourceConfig.validate()`. All switched.
- `@Data` also generated a `toString()` printing url, headers and
open_messages — now
`@ToString.Exclude`.
A credential in the *path* can't be masked without losing the endpoint
identity, so the en/zh
docs now ask for `headers`/`open_messages` in that case.
**2. The 300ms window (Issue 2)** — `[Fix][Connector-V2] Measure the
WebSocket read timeout from the established connection`
You marked this no-action, but following the pointer turned up a production
bug rather than a
test-stability one: `read_timeout_ms` was measured from `reader.open()`,
i.e. *before* the
handshake, so connection setup counted as idle time. If the handshake
outlasts
`read_timeout_ms`, a BATCH job signals no-more-element and finishes
**successfully with zero
rows** — a silent empty result, not a failure. It also contradicted
`connect_timeout_ms`
(default 12000), which explicitly allows a slow handshake. Note the failure
direction is the
opposite of the one in the review: firing *too early*, not later than the
poller expects — late
firing is absorbed by the 30s deadline.
The idle window now starts at the established connection (refreshed on every
successful
reconnect), and the handshake phase never times out. Option description and
en/zh docs updated.
I kept `read_timeout_ms = 300` in the test since, as you say, it is the
value under test — and
the fix widens its margin: previously `300ms − handshake`, now a full 300ms
from connect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
