sadpandajoe commented on code in PR #43689:
URL: https://github.com/apache/superset/pull/43689#discussion_r3888579290
##########
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:
A root-relative websocket endpoint (for example `/superset-ws` behind a
same-origin proxy) is valid for `WebSocket`, but `new URL(url)` rejects it and
this catch connects without `tab_id`. Chart requests still subscribe under the
tab-specific key, so their status events have no matching socket and only
polling notices completion. Could this resolve against the page URL before
adding the parameter?
##########
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:
When a duplicated tab receives `TAB_ID_DENIED`, `useTabId` replaces its
session-storage ID, but this socket remains registered under the old ID until
it happens to reconnect. New chart subscriptions use the replacement ID, so
their tab-targeted status events cannot reach this active socket. Should the
tab-ID change trigger a realtime reconnect or otherwise keep the socket
registration in sync?
--
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]
