Replace RU limit banner by clarifying the error when RU limit is exceeded (#1966)

* allow DE to provide clearer error messages for certain conditions

* allow rendeering a "help" link for an error

* use TableCellLayout where possible

* remove RU Threshold banner, now that we have a clearer error

* refmt

* fix QueryError test

* change "RU Threshold" to "RU Limit"
This commit is contained in:
Ashley Stanton-Nurse 2024-09-12 11:45:10 -07:00 committed by GitHub
parent fdbbbd7378
commit 2c7e788358
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 201 additions and 69 deletions

View File

@ -0,0 +1,94 @@
import QueryError, { QueryErrorLocation, QueryErrorSeverity } from "Common/QueryError";
describe("QueryError.tryParse", () => {
const testErrorLocationResolver = ({ start, end }: { start: number; end: number }) =>
new QueryErrorLocation(
{ offset: start, lineNumber: start, column: start },
{ offset: end, lineNumber: end, column: end },
);
it("handles a string error", () => {
const error = "error";
const result = QueryError.tryParse(error, testErrorLocationResolver);
expect(result).toEqual([new QueryError("error", QueryErrorSeverity.Error)]);
});
it("handles an error object", () => {
const error = {
message: "error",
severity: "Warning",
location: { start: 0, end: 1 },
code: "code",
};
const result = QueryError.tryParse(error, testErrorLocationResolver);
expect(result).toEqual([
new QueryError(
"error",
QueryErrorSeverity.Warning,
"code",
new QueryErrorLocation({ offset: 0, lineNumber: 0, column: 0 }, { offset: 1, lineNumber: 1, column: 1 }),
),
]);
});
it("handles a JSON message without syntax errors", () => {
const innerError = {
code: "BadRequest",
message: "Your query is bad, and you should feel bad",
};
const message = JSON.stringify(innerError);
const outerError = {
code: "BadRequest",
message,
};
const result = QueryError.tryParse(outerError, testErrorLocationResolver);
expect(result).toEqual([
new QueryError("Your query is bad, and you should feel bad", QueryErrorSeverity.Error, "BadRequest"),
]);
});
// Imitate the value coming from the backend, which has the syntax errors serialized as JSON in the message.
it("handles single-nested error", () => {
const errors = [
{
message: "error1",
severity: "Warning",
location: { start: 0, end: 1 },
code: "code1",
},
{
message: "error2",
severity: "Error",
location: { start: 2, end: 3 },
code: "code2",
},
];
const innerError = {
code: "BadRequest",
message: "Your query is bad, and you should feel bad",
errors,
};
const message = JSON.stringify(innerError);
const outerError = {
code: "BadRequest",
message,
};
const result = QueryError.tryParse(outerError, testErrorLocationResolver);
expect(result).toEqual([
new QueryError(
"error1",
QueryErrorSeverity.Warning,
"code1",
new QueryErrorLocation({ offset: 0, lineNumber: 0, column: 0 }, { offset: 1, lineNumber: 1, column: 1 }),
),
new QueryError(
"error2",
QueryErrorSeverity.Error,
"code2",
new QueryErrorLocation({ offset: 2, lineNumber: 2, column: 2 }, { offset: 3, lineNumber: 3, column: 3 }),
),
]);
});
});

View File

@ -1,5 +1,5 @@
import { getErrorMessage } from "Common/ErrorHandlingUtils";
import { monaco } from "Explorer/LazyMonaco"; import { monaco } from "Explorer/LazyMonaco";
import { getRUThreshold, ruThresholdEnabled } from "Shared/StorageUtility";
export enum QueryErrorSeverity { export enum QueryErrorSeverity {
Error = "Error", Error = "Error",
@ -97,13 +97,44 @@ export const createMonacoMarkersForQueryErrors = (errors: QueryError[]) => {
.filter((marker) => !!marker); .filter((marker) => !!marker);
}; };
export interface ErrorEnrichment {
title?: string;
message: string;
learnMoreUrl?: string;
}
const REPLACEMENT_MESSAGES: Record<string, (original: string) => string> = {
OPERATION_RU_LIMIT_EXCEEDED: (original) => {
if (ruThresholdEnabled()) {
const threshold = getRUThreshold();
return `Query exceeded the Request Unit (RU) limit of ${threshold} RUs. You can change this limit in Data Explorer settings.`;
}
return original;
},
};
const HELP_LINKS: Record<string, string> = {
OPERATION_RU_LIMIT_EXCEEDED:
"https://learn.microsoft.com/en-us/azure/cosmos-db/data-explorer#configure-request-unit-threshold",
};
export default class QueryError { export default class QueryError {
message: string;
helpLink?: string;
constructor( constructor(
public message: string, message: string,
public severity: QueryErrorSeverity, public severity: QueryErrorSeverity,
public code?: string, public code?: string,
public location?: QueryErrorLocation, public location?: QueryErrorLocation,
) {} helpLink?: string,
) {
// Automatically replace the message with a more Data Explorer-specific message if we have for this error code.
this.message = REPLACEMENT_MESSAGES[code] ? REPLACEMENT_MESSAGES[code](message) : message;
// Automatically set the help link if we have one for this error code.
this.helpLink = helpLink ?? HELP_LINKS[code];
}
getMonacoSeverity(): monaco.MarkerSeverity { getMonacoSeverity(): monaco.MarkerSeverity {
// It's very difficult to use the monaco.MarkerSeverity enum from here, so we'll just use the numbers directly. // It's very difficult to use the monaco.MarkerSeverity enum from here, so we'll just use the numbers directly.
@ -135,7 +166,7 @@ export default class QueryError {
return errors; return errors;
} }
const errorMessage = getErrorMessage(error as string | Error); const errorMessage = error as string;
// Map some well known messages to richer errors // Map some well known messages to richer errors
const knownError = knownErrors[errorMessage]; const knownError = knownErrors[errorMessage];
@ -160,7 +191,9 @@ export default class QueryError {
} }
const severity = const severity =
"severity" in error && typeof error.severity === "string" ? (error.severity as QueryErrorSeverity) : undefined; "severity" in error && typeof error.severity === "string"
? (error.severity as QueryErrorSeverity)
: QueryErrorSeverity.Error;
const location = const location =
"location" in error && typeof error.location === "object" "location" in error && typeof error.location === "object"
? locationResolver(error.location as { start: number; end: number }) ? locationResolver(error.location as { start: number; end: number })
@ -173,16 +206,15 @@ export default class QueryError {
error: unknown, error: unknown,
locationResolver: (location: { start: number; end: number }) => QueryErrorLocation, locationResolver: (location: { start: number; end: number }) => QueryErrorLocation,
): QueryError[] | null { ): QueryError[] | null {
if (typeof error === "object" && "message" in error) { let message: string | undefined;
error = error.message; if (typeof error === "object" && "message" in error && typeof error.message === "string") {
} message = error.message;
} else {
if (typeof error !== "string") { // Unsupported error format.
return null; return null;
} }
// Assign to a new variable because of a TypeScript flow typing quirk, see below. // Assign to a new variable because of a TypeScript flow typing quirk, see below.
let message = error;
if (message.startsWith("Message: ")) { if (message.startsWith("Message: ")) {
// Reassigning this to 'error' restores the original type of 'error', which is 'unknown'. // Reassigning this to 'error' restores the original type of 'error', which is 'unknown'.
// So we use a separate variable to avoid this. // So we use a separate variable to avoid this.
@ -196,13 +228,16 @@ export default class QueryError {
try { try {
parsed = JSON.parse(message); parsed = JSON.parse(message);
} catch (e) { } catch (e) {
// Not a query error. // The message doesn't contain a nested error.
return null; return [QueryError.read(error, locationResolver)];
} }
if (typeof parsed === "object" && "errors" in parsed && Array.isArray(parsed.errors)) { if (typeof parsed === "object") {
if ("errors" in parsed && Array.isArray(parsed.errors)) {
return parsed.errors.map((e) => QueryError.read(e, locationResolver)).filter((e) => e !== null); return parsed.errors.map((e) => QueryError.read(e, locationResolver)).filter((e) => e !== null);
} }
return [QueryError.read(parsed, locationResolver)];
}
return null; return null;
} }
} }

View File

@ -608,16 +608,16 @@ export const SettingsPane: FunctionComponent<{ explorer: Explorer }> = ({
<AccordionItem value="4"> <AccordionItem value="4">
<AccordionHeader> <AccordionHeader>
<div className={styles.header}>RU Threshold</div> <div className={styles.header}>RU Limit</div>
</AccordionHeader> </AccordionHeader>
<AccordionPanel> <AccordionPanel>
<div className={styles.settingsSectionContainer}> <div className={styles.settingsSectionContainer}>
<div className={styles.settingsSectionDescription}> <div className={styles.settingsSectionDescription}>
If a query exceeds a configured RU threshold, the query will be aborted. If a query exceeds a configured RU limit, the query will be aborted.
</div> </div>
<Toggle <Toggle
styles={toggleStyles} styles={toggleStyles}
label="Enable RU threshold" label="Enable RU limit"
onChange={handleOnRUThresholdToggleChange} onChange={handleOnRUThresholdToggleChange}
defaultChecked={ruThresholdEnabled} defaultChecked={ruThresholdEnabled}
/> />
@ -625,7 +625,7 @@ export const SettingsPane: FunctionComponent<{ explorer: Explorer }> = ({
{ruThresholdEnabled && ( {ruThresholdEnabled && (
<div className={styles.settingsSectionContainer}> <div className={styles.settingsSectionContainer}>
<SpinButton <SpinButton
label="RU Threshold (RU)" label="RU Limit (RU)"
labelPosition={Position.top} labelPosition={Position.top}
defaultValue={(ruThreshold || DefaultRUThreshold).toString()} defaultValue={(ruThreshold || DefaultRUThreshold).toString()}
min={1} min={1}

View File

@ -154,7 +154,7 @@ exports[`Settings Pane should render Default properly 1`] = `
<div <div
className="___15c001r_0000000 fq02s40" className="___15c001r_0000000 fq02s40"
> >
RU Threshold RU Limit
</div> </div>
</AccordionHeader> </AccordionHeader>
<AccordionPanel> <AccordionPanel>
@ -164,11 +164,11 @@ exports[`Settings Pane should render Default properly 1`] = `
<div <div
className="___10gar1i_0000000 f1fow5ox f1ugzwwg" className="___10gar1i_0000000 f1fow5ox f1ugzwwg"
> >
If a query exceeds a configured RU threshold, the query will be aborted. If a query exceeds a configured RU limit, the query will be aborted.
</div> </div>
<StyledToggleBase <StyledToggleBase
defaultChecked={true} defaultChecked={true}
label="Enable RU threshold" label="Enable RU limit"
onChange={[Function]} onChange={[Function]}
styles={ styles={
{ {
@ -193,7 +193,7 @@ exports[`Settings Pane should render Default properly 1`] = `
decrementButtonAriaLabel="Decrease value by 1000" decrementButtonAriaLabel="Decrease value by 1000"
defaultValue="5000" defaultValue="5000"
incrementButtonAriaLabel="Increase value by 1000" incrementButtonAriaLabel="Increase value by 1000"
label="RU Threshold (RU)" label="RU Limit (RU)"
labelPosition={0} labelPosition={0}
min={1} min={1}
onChange={[Function]} onChange={[Function]}

View File

@ -12,7 +12,7 @@ import {
createTableColumn, createTableColumn,
tokens, tokens,
} from "@fluentui/react-components"; } from "@fluentui/react-components";
import { ErrorCircleFilled, MoreHorizontalRegular, WarningFilled } from "@fluentui/react-icons"; import { ErrorCircleFilled, MoreHorizontalRegular, QuestionRegular, WarningFilled } from "@fluentui/react-icons";
import QueryError, { QueryErrorSeverity, compareSeverity } from "Common/QueryError"; import QueryError, { QueryErrorSeverity, compareSeverity } from "Common/QueryError";
import { useQueryTabStyles } from "Explorer/Tabs/QueryTab/Styles"; import { useQueryTabStyles } from "Explorer/Tabs/QueryTab/Styles";
import { useNotificationConsole } from "hooks/useNotificationConsole"; import { useNotificationConsole } from "hooks/useNotificationConsole";
@ -34,25 +34,32 @@ export const ErrorList: React.FC<{ errors: QueryError[] }> = ({ errors }) => {
createTableColumn<QueryError>({ createTableColumn<QueryError>({
columnId: "code", columnId: "code",
compare: (item1, item2) => item1.code.localeCompare(item2.code), compare: (item1, item2) => item1.code.localeCompare(item2.code),
renderHeaderCell: () => null, renderHeaderCell: () => "Code",
renderCell: (item) => item.code, renderCell: (item) => <TableCellLayout truncate>{item.code}</TableCellLayout>,
}), }),
createTableColumn<QueryError>({ createTableColumn<QueryError>({
columnId: "severity", columnId: "severity",
compare: (item1, item2) => compareSeverity(item1.severity, item2.severity), compare: (item1, item2) => compareSeverity(item1.severity, item2.severity),
renderHeaderCell: () => null, renderHeaderCell: () => "Severity",
renderCell: (item) => <TableCellLayout media={severityIcons[item.severity]}>{item.severity}</TableCellLayout>, renderCell: (item) => (
<TableCellLayout truncate media={severityIcons[item.severity]}>
{item.severity}
</TableCellLayout>
),
}), }),
createTableColumn<QueryError>({ createTableColumn<QueryError>({
columnId: "location", columnId: "location",
compare: (item1, item2) => item1.location?.start?.offset - item2.location?.start?.offset, compare: (item1, item2) => item1.location?.start?.offset - item2.location?.start?.offset,
renderHeaderCell: () => "Location", renderHeaderCell: () => "Location",
renderCell: (item) => renderCell: (item) => (
item.location <TableCellLayout truncate>
{item.location
? item.location.start.lineNumber ? item.location.start.lineNumber
? `Line ${item.location.start.lineNumber}` ? `Line ${item.location.start.lineNumber}`
: "<unknown>" : "<unknown>"
: "<no location>", : "<no location>"}
</TableCellLayout>
),
}), }),
createTableColumn<QueryError>({ createTableColumn<QueryError>({
columnId: "message", columnId: "message",
@ -60,8 +67,20 @@ export const ErrorList: React.FC<{ errors: QueryError[] }> = ({ errors }) => {
renderHeaderCell: () => "Message", renderHeaderCell: () => "Message",
renderCell: (item) => ( renderCell: (item) => (
<div className={styles.errorListMessageCell}> <div className={styles.errorListMessageCell}>
<div className={styles.errorListMessage}>{item.message}</div> <div className={styles.errorListMessage} title={item.message}>
<div> {item.message}
</div>
<div className={styles.errorListMessageActions}>
{item.helpLink && (
<Button
as="a"
aria-label="Help"
appearance="subtle"
icon={<QuestionRegular />}
href={item.helpLink}
target="_blank"
/>
)}
<Button <Button
aria-label="Details" aria-label="Details"
appearance="subtle" appearance="subtle"
@ -76,9 +95,9 @@ export const ErrorList: React.FC<{ errors: QueryError[] }> = ({ errors }) => {
const columnSizingOptions: TableColumnSizingOptions = { const columnSizingOptions: TableColumnSizingOptions = {
code: { code: {
minWidth: 75, minWidth: 90,
idealWidth: 75, idealWidth: 90,
defaultWidth: 75, defaultWidth: 90,
}, },
severity: { severity: {
minWidth: 100, minWidth: 100,

View File

@ -72,6 +72,11 @@ export const useQueryTabStyles = makeStyles({
metricsGridButtons: { metricsGridButtons: {
...cosmosShorthands.borderTop(), ...cosmosShorthands.borderTop(),
}, },
errorListTableCell: {
textOverflow: "ellipsis",
whiteSpace: "nowrap",
overflow: "hidden",
},
errorListMessageCell: { errorListMessageCell: {
display: "flex", display: "flex",
flexDirection: "row", flexDirection: "row",
@ -80,5 +85,12 @@ export const useQueryTabStyles = makeStyles({
}, },
errorListMessage: { errorListMessage: {
flexGrow: 1, flexGrow: 1,
textOverflow: "ellipsis",
whiteSpace: "nowrap",
overflow: "hidden",
},
errorListMessageActions: {
display: "flex",
flexDirection: "row",
}, },
}); });

View File

@ -1,7 +1,7 @@
import { IMessageBarStyles, Link, MessageBar, MessageBarButton, MessageBarType } from "@fluentui/react"; import { IMessageBarStyles, MessageBar, MessageBarButton, MessageBarType } from "@fluentui/react";
import { CassandraProxyEndpoints, MongoProxyEndpoints } from "Common/Constants"; import { CassandraProxyEndpoints, MongoProxyEndpoints } from "Common/Constants";
import { sendMessage } from "Common/MessageHandler"; import { sendMessage } from "Common/MessageHandler";
import { Platform, configContext, updateConfigContext } from "ConfigContext"; import { configContext, updateConfigContext } from "ConfigContext";
import { IpRule } from "Contracts/DataModels"; import { IpRule } from "Contracts/DataModels";
import { MessageTypes } from "Contracts/ExplorerContracts"; import { MessageTypes } from "Contracts/ExplorerContracts";
import { CollectionTabKind } from "Contracts/ViewModels"; import { CollectionTabKind } from "Contracts/ViewModels";
@ -16,7 +16,6 @@ import { VcoreMongoConnectTab } from "Explorer/Tabs/VCoreMongoConnectTab";
import { VcoreMongoQuickstartTab } from "Explorer/Tabs/VCoreMongoQuickstartTab"; import { VcoreMongoQuickstartTab } from "Explorer/Tabs/VCoreMongoQuickstartTab";
import { LayoutConstants } from "Explorer/Theme/ThemeUtil"; import { LayoutConstants } from "Explorer/Theme/ThemeUtil";
import { KeyboardAction, KeyboardActionGroup, useKeyboardActionGroup } from "KeyboardShortcuts"; import { KeyboardAction, KeyboardActionGroup, useKeyboardActionGroup } from "KeyboardShortcuts";
import { hasRUThresholdBeenConfigured } from "Shared/StorageUtility";
import { userContext } from "UserContext"; import { userContext } from "UserContext";
import { CassandraProxyOutboundIPs, MongoProxyOutboundIPs, PortalBackendIPs } from "Utils/EndpointUtils"; import { CassandraProxyOutboundIPs, MongoProxyOutboundIPs, PortalBackendIPs } from "Utils/EndpointUtils";
import { useTeachingBubble } from "hooks/useTeachingBubble"; import { useTeachingBubble } from "hooks/useTeachingBubble";
@ -37,9 +36,6 @@ interface TabsProps {
export const Tabs = ({ explorer }: TabsProps): JSX.Element => { export const Tabs = ({ explorer }: TabsProps): JSX.Element => {
const { openedTabs, openedReactTabs, activeTab, activeReactTab, networkSettingsWarning } = useTabs(); const { openedTabs, openedReactTabs, activeTab, activeReactTab, networkSettingsWarning } = useTabs();
const [showRUThresholdMessageBar, setShowRUThresholdMessageBar] = useState<boolean>(
userContext.apiType === "SQL" && configContext.platform !== Platform.Fabric && !hasRUThresholdBeenConfigured(),
);
const [ const [
showMongoAndCassandraProxiesNetworkSettingsWarningState, showMongoAndCassandraProxiesNetworkSettingsWarningState,
setShowMongoAndCassandraProxiesNetworkSettingsWarningState, setShowMongoAndCassandraProxiesNetworkSettingsWarningState,
@ -87,30 +83,6 @@ export const Tabs = ({ explorer }: TabsProps): JSX.Element => {
{networkSettingsWarning} {networkSettingsWarning}
</MessageBar> </MessageBar>
)} )}
{showRUThresholdMessageBar && (
<MessageBar
messageBarType={MessageBarType.info}
onDismiss={() => {
setShowRUThresholdMessageBar(false);
}}
styles={{
...defaultMessageBarStyles,
innerText: {
fontWeight: "bold",
},
}}
dismissButtonAriaLabel="Close info"
>
{`Data Explorer has a 5,000 RU default limit. To adjust the limit, go to the Settings page and find "RU Threshold".`}
<Link
className="underlinedLink"
href="https://review.learn.microsoft.com/en-us/azure/cosmos-db/data-explorer?branch=main#configure-request-unit-threshold"
target="_blank"
>
Learn More
</Link>
</MessageBar>
)}
{showMongoAndCassandraProxiesNetworkSettingsWarningState && ( {showMongoAndCassandraProxiesNetworkSettingsWarningState && (
<MessageBar <MessageBar
messageBarType={MessageBarType.warning} messageBarType={MessageBarType.warning}