villebro commented on code in PR #43689:
URL: https://github.com/apache/superset/pull/43689#discussion_r3889909150


##########
superset-frontend/src/middleware/realtime.ts:
##########
@@ -98,9 +99,21 @@ export const dispatchRealtimeMessage = (rawData: string): 
void => {
 const openSocket = (thisGeneration: number): void => {
   if (thisGeneration !== generation) return;
   if (!enabled || !url || typeof WebSocket === 'undefined') return;
+  // Advertise this tab's id on the connect URL so the server can also bind the
+  // socket to a per-tab channel and deliver tab-targeted messages (e.g. this
+  // tab's own task-status) to it alone. Built per-connection so the stored 
base
+  // `url` stays stable for connectRealtime's idempotency check.
+  let connectUrl = url;
+  try {
+    const parsed = new URL(url);

Review Comment:
   Good catch — fixed in a371d2103d. `openSocket` now builds the connect URL 
with `new URL(url, window.location.href)`, so a root-relative endpoint (e.g. 
`/superset-ws` behind a same-origin proxy) resolves against the page and still 
carries `tab_id`; an `http(s)` result is normalized to `ws(s)`. Absolute 
`ws(s)://` URLs are unaffected (an absolute URL ignores the base). Added a test 
(`resolves a root-relative ws url against the page and adds tab_id`).



##########
superset-frontend/src/middleware/realtime.ts:
##########
@@ -98,9 +99,21 @@ export const dispatchRealtimeMessage = (rawData: string): 
void => {
 const openSocket = (thisGeneration: number): void => {
   if (thisGeneration !== generation) return;
   if (!enabled || !url || typeof WebSocket === 'undefined') return;
+  // Advertise this tab's id on the connect URL so the server can also bind the
+  // socket to a per-tab channel and deliver tab-targeted messages (e.g. this
+  // tab's own task-status) to it alone. Built per-connection so the stored 
base
+  // `url` stays stable for connectRealtime's idempotency check.
+  let connectUrl = url;
+  try {
+    const parsed = new URL(url);
+    parsed.searchParams.set('tab_id', getTabId());

Review Comment:
   Fixed in a371d2103d — the tab-id change now triggers a realtime reconnect so 
the socket re-registers under the new per-tab channel. `useTabId` exposes 
`subscribeTabIdChange` and calls it on the `TAB_ID_DENIED` reassignment; 
`realtime.ts` subscribes and, when a socket is active, tears down and reopens 
(which re-reads `getTabId()`), so new tab-targeted status events reach the 
current socket instead of only being noticed by polling. Test: `reconnects with 
the new tab id when the tab id changes`.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to