mirror of
https://github.com/Azure/cosmos-explorer.git
synced 2026-09-20 01:22:47 +01:00
Address review: do not trade false-unhealthy for false-healthy
The previous revision removed the missing-phase signatures, but three of those changes could report a load as healthy without establishing that it finished, and one could prevent a slow initialization from recovering. Preserve late initialization recovery. configurePortal() raced the iframe handshake against a 30s reject while leaving the message listener active, so a late init message would build an Explorer that the rejected caller never received — turning a slow load into an unrecoverable one. The timeout is now a watchdog that traces the stuck handshake and lets the caller keep waiting, so recovery behaves as it did before. Initialization work inside the message handler is wrapped so a throw reports and still yields a shell, instead of leaving the promise pending forever. Do not accept an earlier render as proof the loaded tree is ready. Phase auto-start let a render of the databases-only tree complete DatabaseTreeRendered with a zero-duration measurement, before collections had loaded. The producer ordering is fixed instead: Explorer publishes a ready revision once the load has produced the data the tree should show, and ResourceTree completes the phase only for a render carrying that revision, acknowledging each revision once. Stale callbacks from a superseded load, accounts with no databases and unchanged trees are covered by tests. A completion for a phase that was never opened is now refused and reported as phase_complete_unstarted rather than backdated, and the early-completion buffer no longer accepts deferred phases, which must be opened by their producer. Do not turn unfinished background loads into successes. A timeout in a hidden tab emitted healthy=true even with no phases completed. The emitted outcome again reflects what actually happened; documentHidden continues to be reported so alerting can apply a background policy without the load being relabelled. Related: IcM 865096261
This commit is contained in:
Dmitrii Shilov
@@ -710,11 +710,12 @@ export async function fetchAndUpdateKeys(subscriptionId: string, resourceGroup:
|
||||
}
|
||||
|
||||
// The portal configures Data Explorer with a single iframe message. If a dependency on the
|
||||
// portal side never resolves, that message is never posted — and without this timeout the
|
||||
// promise below stays pending forever, leaving the user on <LoadingExplorer /> with no
|
||||
// diagnostic while the ApplicationLoad health scenario times out mute (IcM 865096261).
|
||||
// Deliberately well above the 10s scenario budget so only genuinely stuck handshakes fail.
|
||||
const PORTAL_INIT_MESSAGE_TIMEOUT_MS = 30000;
|
||||
// portal side never resolves, that message is never posted and the promise below stays
|
||||
// pending, leaving the user on <LoadingExplorer /> while the ApplicationLoad health scenario
|
||||
// times out with no diagnostic (IcM 865096261). The watchdog below reports that, but
|
||||
// deliberately does not reject: a late init message must still be able to complete setup.
|
||||
// Well above the 10s scenario budget so only genuinely stuck handshakes are reported.
|
||||
const PORTAL_INIT_MESSAGE_WATCHDOG_MS = 30000;
|
||||
|
||||
async function configurePortal(): Promise<Explorer> {
|
||||
const configureStartKey = traceStart(Action.ConfigurePortal, {
|
||||
@@ -725,7 +726,7 @@ async function configurePortal(): Promise<Explorer> {
|
||||
});
|
||||
|
||||
let explorer: Explorer;
|
||||
let initMessageTimeoutId: number;
|
||||
let initMessageReceived = false;
|
||||
|
||||
const explorerReady = new Promise<Explorer>((resolve) => {
|
||||
// In development mode, try to load the iframe message from session storage.
|
||||
@@ -767,76 +768,90 @@ async function configurePortal(): Promise<Explorer> {
|
||||
const inputs = message?.inputs;
|
||||
const openAction = message?.openAction;
|
||||
if (inputs) {
|
||||
updateContextsFromPortalMessage(inputs);
|
||||
initMessageReceived = true;
|
||||
try {
|
||||
updateContextsFromPortalMessage(inputs);
|
||||
|
||||
const { databaseAccount: account, subscriptionId, resourceGroup } = userContext;
|
||||
const { databaseAccount: account, subscriptionId, resourceGroup } = userContext;
|
||||
|
||||
if (userContext.apiType === "SQL") {
|
||||
checkAndUpdateSelectedRegionalEndpoint();
|
||||
}
|
||||
|
||||
let dataPlaneRbacEnabled;
|
||||
if (isDataplaneRbacSupported(userContext.apiType)) {
|
||||
if (LocalStorageUtility.hasItem(StorageKey.DataPlaneRbacEnabled)) {
|
||||
const isDataPlaneRbacSetting = LocalStorageUtility.getEntryString(StorageKey.DataPlaneRbacEnabled);
|
||||
Logger.logInfo(
|
||||
`Local storage RBAC setting for ${userContext.apiType} account ${account.name} is ${isDataPlaneRbacSetting}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
|
||||
if (isDataPlaneRbacSetting === Constants.RBACOptions.setAutomaticRBACOption) {
|
||||
dataPlaneRbacEnabled = account.properties.disableLocalAuth;
|
||||
} else {
|
||||
dataPlaneRbacEnabled = isDataPlaneRbacSetting === Constants.RBACOptions.setTrueRBACOption;
|
||||
}
|
||||
} else {
|
||||
Logger.logInfo(
|
||||
`Local storage does not exist for ${userContext.apiType} account ${account.name} with disable local auth set to ${account.properties.disableLocalAuth} is ${dataPlaneRbacEnabled}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
dataPlaneRbacEnabled = account.properties.disableLocalAuth;
|
||||
if (userContext.apiType === "SQL") {
|
||||
checkAndUpdateSelectedRegionalEndpoint();
|
||||
}
|
||||
Logger.logInfo(
|
||||
`Data Plane RBAC value for ${userContext.apiType} account ${account.name} with disable local auth set to ${account.properties.disableLocalAuth} is ${dataPlaneRbacEnabled}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
|
||||
if (!dataPlaneRbacEnabled) {
|
||||
let dataPlaneRbacEnabled;
|
||||
if (isDataplaneRbacSupported(userContext.apiType)) {
|
||||
if (LocalStorageUtility.hasItem(StorageKey.DataPlaneRbacEnabled)) {
|
||||
const isDataPlaneRbacSetting = LocalStorageUtility.getEntryString(StorageKey.DataPlaneRbacEnabled);
|
||||
Logger.logInfo(
|
||||
`Local storage RBAC setting for ${userContext.apiType} account ${account.name} is ${isDataPlaneRbacSetting}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
|
||||
if (isDataPlaneRbacSetting === Constants.RBACOptions.setAutomaticRBACOption) {
|
||||
dataPlaneRbacEnabled = account.properties.disableLocalAuth;
|
||||
} else {
|
||||
dataPlaneRbacEnabled = isDataPlaneRbacSetting === Constants.RBACOptions.setTrueRBACOption;
|
||||
}
|
||||
} else {
|
||||
Logger.logInfo(
|
||||
`Local storage does not exist for ${userContext.apiType} account ${account.name} with disable local auth set to ${account.properties.disableLocalAuth} is ${dataPlaneRbacEnabled}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
dataPlaneRbacEnabled = account.properties.disableLocalAuth;
|
||||
}
|
||||
Logger.logInfo(
|
||||
`Data Plane RBAC value for ${userContext.apiType} account ${account.name} with disable local auth set to ${account.properties.disableLocalAuth} is ${dataPlaneRbacEnabled}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
|
||||
if (!dataPlaneRbacEnabled) {
|
||||
Logger.logInfo(
|
||||
`Calling fetch keys for ${userContext.apiType} account ${account.name}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
await fetchAndUpdateKeys(subscriptionId, resourceGroup, account.name);
|
||||
} else {
|
||||
Logger.logInfo(
|
||||
`Trying to silently acquire MSAL token for ${userContext.apiType} account ${account.name}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
try {
|
||||
const aadToken = await acquireMsalTokenForAccount(userContext.databaseAccount, true);
|
||||
updateUserContext({ aadToken: aadToken });
|
||||
useDataPlaneRbac.setState({ aadTokenUpdated: true });
|
||||
} catch (authError) {
|
||||
Logger.logWarning(
|
||||
`Failed to silently acquire authorization token from MSAL: ${authError} for ${userContext.apiType} account ${account}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
logConsoleError("Failed to silently acquire authorization token: " + authError);
|
||||
}
|
||||
}
|
||||
|
||||
updateUserContext({ dataPlaneRbacEnabled });
|
||||
useDataPlaneRbac.setState({ dataPlaneRbacEnabled: dataPlaneRbacEnabled });
|
||||
} else if (userContext.apiType !== "Postgres" && userContext.apiType !== "VCoreMongo") {
|
||||
Logger.logInfo(
|
||||
`Calling fetch keys for ${userContext.apiType} account ${account.name}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
await fetchAndUpdateKeys(subscriptionId, resourceGroup, account.name);
|
||||
} else {
|
||||
Logger.logInfo(
|
||||
`Trying to silently acquire MSAL token for ${userContext.apiType} account ${account.name}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
try {
|
||||
const aadToken = await acquireMsalTokenForAccount(userContext.databaseAccount, true);
|
||||
updateUserContext({ aadToken: aadToken });
|
||||
useDataPlaneRbac.setState({ aadTokenUpdated: true });
|
||||
} catch (authError) {
|
||||
Logger.logWarning(
|
||||
`Failed to silently acquire authorization token from MSAL: ${authError} for ${userContext.apiType} account ${account}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
logConsoleError("Failed to silently acquire authorization token: " + authError);
|
||||
}
|
||||
}
|
||||
|
||||
updateUserContext({ dataPlaneRbacEnabled });
|
||||
useDataPlaneRbac.setState({ dataPlaneRbacEnabled: dataPlaneRbacEnabled });
|
||||
} else if (userContext.apiType !== "Postgres" && userContext.apiType !== "VCoreMongo") {
|
||||
Logger.logInfo(
|
||||
`Calling fetch keys for ${userContext.apiType} account ${account.name}`,
|
||||
"Explorer/configurePortal",
|
||||
);
|
||||
await fetchAndUpdateKeys(subscriptionId, resourceGroup, account.name);
|
||||
explorer = new Explorer();
|
||||
traceSuccess(Action.ConfigurePortal, {}, configureStartKey);
|
||||
} catch (error) {
|
||||
// A valid init message arrived but setup threw. Leaving the promise pending would
|
||||
// strand the user on <LoadingExplorer /> with nothing reported, so the shell is
|
||||
// still created below: steps depending on the failed work surface their own errors.
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
traceFailure(Action.ConfigurePortal, { error: errorMessage }, configureStartKey);
|
||||
logConsoleError(`Data Explorer initialization failed: ${errorMessage}`);
|
||||
}
|
||||
|
||||
explorer = new Explorer();
|
||||
traceSuccess(Action.ConfigurePortal, {}, configureStartKey);
|
||||
if (!explorer) {
|
||||
explorer = new Explorer();
|
||||
}
|
||||
resolve(explorer);
|
||||
|
||||
if (openAction) {
|
||||
@@ -863,20 +878,27 @@ async function configurePortal(): Promise<Explorer> {
|
||||
sendReadyMessage();
|
||||
});
|
||||
|
||||
const initMessageTimeout = new Promise<never>((_resolve, reject) => {
|
||||
initMessageTimeoutId = window.setTimeout(() => {
|
||||
const error = new Error(
|
||||
`Portal did not send the Data Explorer init message within ${PORTAL_INIT_MESSAGE_TIMEOUT_MS}ms`,
|
||||
);
|
||||
traceFailure(Action.ConfigurePortal, { error: error.message }, configureStartKey);
|
||||
reject(error);
|
||||
}, PORTAL_INIT_MESSAGE_TIMEOUT_MS);
|
||||
});
|
||||
// Report a handshake that has not produced an init message, without cancelling it. Rejecting
|
||||
// here would abandon the caller while the listener stays live, so a late message would build
|
||||
// an Explorer that setExplorer() never receives — the load would be unrecoverable rather
|
||||
// than merely slow.
|
||||
const watchdogId = window.setTimeout(() => {
|
||||
if (initMessageReceived) {
|
||||
return;
|
||||
}
|
||||
traceFailure(
|
||||
Action.ConfigurePortal,
|
||||
{
|
||||
error: `Portal has not sent the Data Explorer init message after ${PORTAL_INIT_MESSAGE_WATCHDOG_MS}ms; still waiting`,
|
||||
},
|
||||
configureStartKey,
|
||||
);
|
||||
}, PORTAL_INIT_MESSAGE_WATCHDOG_MS);
|
||||
|
||||
try {
|
||||
return await Promise.race([explorerReady, initMessageTimeout]);
|
||||
return await explorerReady;
|
||||
} finally {
|
||||
window.clearTimeout(initMessageTimeoutId);
|
||||
window.clearTimeout(watchdogId);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user