Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use Critical error dialog with context #4942

Merged
merged 11 commits into from
Aug 9, 2023
Merged

Use Critical error dialog with context #4942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
38953ee
webui: add button for reporting to Bugzilla to CriticalError dialog
rvykydal Jul 25, 2023
73d8734
webui: use form layout and add log review of journal
rvykydal Aug 8, 2023
725b011
webui: display also the background wizard page with Critical Error di…
rvykydal Aug 1, 2023
e7e9879
webui: use Critical Error dialog to handle erros in getting data abou…
rvykydal Jul 26, 2023
cf7f04d
webui: use Critical Error dialog in installation progress
rvykydal Jul 27, 2023
4f83d0f
webui: use Critical Error dialog for critical failures on Back button
rvykydal Jul 27, 2023
9544839
webui: translate critical error context only in the dialog
rvykydal Aug 4, 2023
a9c352b
webui: add testing of log existence in critical error handling
rvykydal Aug 7, 2023
bcbb000
webui: give better feedback to user after 'Report issue' was clicked
KKoukiou Aug 8, 2023
25566d9
webui: tests: cleanup webui.log in the end of the test
KKoukiou Aug 8, 2023
7208c42
webui: in Critical Error test click to report to BZ only after the lo…
rvykydal Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 10 additions & 3 deletions ui/webui/src/components/AnacondaWizard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
} from "../apis/storage.js";

const _ = cockpit.gettext;
const N_ = cockpit.noop;

export const AnacondaWizard = ({ dispatch, isBootIso, osRelease, storageData, localizationData, onCritFail, onAddErrorNotification, title, conf }) => {
const [isFormValid, setIsFormValid] = useState(false);
Expand Down Expand Up @@ -215,6 +216,7 @@ export const AnacondaWizard = ({ dispatch, isBootIso, osRelease, storageData, lo
<Wizard
id="installation-wizard"
footer={<Footer
onCritFail={onCritFail}
isFormValid={isFormValid}
partitioning={storageData.partitioning?.path}
setIsFormValid={setIsFormValid}
Expand All @@ -239,6 +241,7 @@ export const AnacondaWizard = ({ dispatch, isBootIso, osRelease, storageData, lo
};

const Footer = ({
onCritFail,
isFormValid,
setIsFormValid,
setStepNotification,
Expand Down Expand Up @@ -302,14 +305,14 @@ const Footer = ({
}
};

const goToPreviousStep = (activeStep, onBack) => {
const goToPreviousStep = (activeStep, onBack, errorHandler) => {
// first reset validation state to default
setIsFormValid(true);
onBack();
if (activeStep.id === "installation-review") {
resetPartitioning().then(() => {
debug("resetPartitioning");
}, console.error);
}, errorHandler);
KKoukiou marked this conversation as resolved.
Show resolved Hide resolved
}
};

Expand Down Expand Up @@ -373,7 +376,11 @@ const Footer = ({
id="installation-back-btn"
variant="secondary"
isDisabled={isFirstScreen}
onClick={() => goToPreviousStep(activeStep, onBack)}>
onClick={() => goToPreviousStep(
activeStep,
onBack,
onCritFail({ context: cockpit.format(N_("Error was hit when going back from $0."), activeStep.name) })
)}>
{_("Back")}
</Button>
<Button
Expand Down
122 changes: 101 additions & 21 deletions ui/webui/src/components/Error.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,55 +16,135 @@
*/

import cockpit from "cockpit";
import React from "react";
import React, { useEffect, useState } from "react";

import {
Button,
Form,
FormGroup,
FormHelperText,
HelperText,
HelperTextItem,
Modal,
ModalVariant,
TextArea,
TextContent,
TextVariants,
Text,
} from "@patternfly/react-core";
import { ExternalLinkAltIcon } from "@patternfly/react-icons";

import { exitGui } from "../helpers/exit.js";

const _ = cockpit.gettext;

export const CriticalError = ({ exception, isBootIso }) => {
export const bugzillaPrefiledReportURL = (productQueryData) => {
const baseURL = "https://bugzilla.redhat.com";
const queryData = {
...productQueryData,
component: "anaconda",
};

const reportURL = new URL(baseURL);
reportURL.pathname = "enter_bug.cgi";
Object.keys(queryData).map(query => reportURL.searchParams.append(query, queryData[query]));
return reportURL.href;
};

const addExceptionDataToReportURL = (url, exception) => {
const newUrl = new URL(url);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we don't check for maximum URL length ? Could be something for a followup PR, possibly with corresponding policy for what to cut if we reach the maximum length.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a Jira issue for that BTW. :)


const context = exception.contextData?.context ? exception.contextData?.context + " " : "";

newUrl.searchParams.append(
"short_desc",
"WebUI: " + context + exception.name + ": " + exception.message
);
newUrl.searchParams.append(
"comment",
"Installer WebUI Critical Error:\n" + context + exception.name + ": " + exception.message + "\n\n" + _("Please attach the file /tmp/webui.log to the issue.")
);
return newUrl.href;
};

export const CriticalError = ({ exception, isBootIso, reportLinkURL }) => {
const reportURL = addExceptionDataToReportURL(reportLinkURL, exception);
const [logContent, setLogContent] = useState("");
const [preparingReport, setPreparingReport] = useState(false);

useEffect(() => {
cockpit.spawn(["journalctl", "-a"])
.then(content => setLogContent(content));
}, []);

const openBZIssue = (reportURL) => {
setPreparingReport(true);
cockpit
.file("/tmp/webui.log")
.replace(logContent)
.always(() => setPreparingReport(false))
.then(() => window.open(reportURL, "_blank", "noopener,noreferer"));
};

const context = exception.contextData?.context;

return (
<Modal
description={_("The installer cannot continue due to a critical error.")}
description={context
? cockpit.format(_("The installer cannot continue due to a critical error: $0"), _(context))
: _("The installer cannot continue due to a critical error.")}
id="critical-error-modal"
isOpen
position="top"
showClose={false}
title={_("Critical error")}
titleIconVariant="danger"
variant="small"
variant={ModalVariant.large}
footer={
<>
{reportLinkURL &&
<Button
variant="primary"
isLoading={preparingReport}
isDisabled={preparingReport}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should disable this button instead of adding sleep in the test. So isDisabled={!content || preparingReport}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open a followup PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5028

icon={<ExternalLinkAltIcon />}
onClick={() => openBZIssue(reportURL)}
component="a">
{preparingReport ? _("Preparing report") : _("Report issue")}
</Button>}
<Button variant="secondary" onClick={exitGui}>
{isBootIso ? _("Reboot") : _("Quit")}
</Button>
</>
}>
{exception.contextData?.context &&
<TextContent>
<Text component={TextVariants.p}>
{cockpit.format(_("Action: $0"), exception.contextData.context)}
</Text>
</TextContent>}
<TextContent>
<Text component={TextVariants.p}>
{cockpit.format(_("Error: $0"), exception.message)}
</Text>
</TextContent>
{exception.contextData?.hint &&
<TextContent>
<Text component={TextVariants.p}>
{cockpit.format(_("Hint: $0"), exception.contextData.hint)}
</Text>
</TextContent>}
<Form>
<FormGroup
fieldId="critical-error-review-details"
label={_("Error details")}
>
<TextContent id="critical-error-review-details">
<Text component={TextVariants.p}>
{exception.name + ": " + exception.message}
</Text>
</TextContent>
</FormGroup>
<FormGroup
KKoukiou marked this conversation as resolved.
Show resolved Hide resolved
fieldId="critical-error-review-attached-log"
label={_("Log")}
>
<TextArea
value={logContent}
onChange={setLogContent}
resizeOrientation="vertical"
id="critical-error-review-attached-log"
/>
<FormHelperText isHidden={false}>
<HelperText>
<HelperTextItem>{_("Reporting an issue will send information over the network. Plese review and edit the attached log to remove any sensitive information.")}</HelperTextItem>
</HelperText>
</FormHelperText>
</FormGroup>
</Form>
</Modal>
);
};
Expand Down
117 changes: 62 additions & 55 deletions ui/webui/src/components/app.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { WithDialogs } from "dialogs.jsx";
import { AddressContext, LanguageContext } from "./Common.jsx";
import { AnacondaHeader } from "./AnacondaHeader.jsx";
import { AnacondaWizard } from "./AnacondaWizard.jsx";
import { CriticalError, errorHandlerWithContext } from "./Error.jsx";
import { CriticalError, errorHandlerWithContext, bugzillaPrefiledReportURL } from "./Error.jsx";

import { BossClient } from "../apis/boss.js";
import { LocalizationClient, initDataLocalization, startEventMonitorLocalization } from "../apis/localization.js";
Expand All @@ -42,6 +42,7 @@ import { debug } from "../helpers/log.js";
import { useReducerWithThunk, reducer, initialState } from "../reducer.js";

const _ = cockpit.gettext;
const N_ = cockpit.noop;

export const Application = () => {
const [address, setAddress] = useState();
Expand Down Expand Up @@ -82,17 +83,17 @@ export const Application = () => {
setStoreInitialized(true);
startEventMonitorStorage({ dispatch });
startEventMonitorLocalization({ dispatch });
}, onCritFail({ context: _("Reading information about the computer failed.") }));
}, onCritFail({ context: N_("Reading information about the computer failed.") }));

getIsFinal().then(
isFinal => setBeta(!isFinal),
onCritFail({ context: _("Reading installer version information failed.") })
onCritFail({ context: N_("Reading installer version information failed.") })
);
});

readConf().then(
setConf,
onCritFail({ context: _("Reading installer configuration failed.") })
onCritFail({ context: N_("Reading installer configuration failed.") })
);

readOsRelease().then(osRelease => setOsRelease(osRelease));
Expand All @@ -119,59 +120,65 @@ export const Application = () => {
const isBootIso = conf?.["Installation System"].type === "BOOT_ISO";
const title = cockpit.format(_("$0 installation"), osRelease.PRETTY_NAME);

const bzReportURL = bugzillaPrefiledReportURL({
product: osRelease.REDHAT_BUGZILLA_PRODUCT,
version: osRelease.REDHAT_BUGZILLA_PRODUCT_VERSION,
});

const page = (
criticalError
? <CriticalError exception={criticalError} isBootIso={isBootIso} />
: (
<Page
data-debug={conf.Anaconda.debug}
additionalGroupedContent={
<AnacondaHeader beta={beta} title={title} />
}
groupProps={{
sticky: "top"
}}
>
{Object.keys(notifications).length > 0 &&
<AlertGroup isToast isLiveRegion>
{Object.keys(notifications).map(idx => {
const notification = notifications[idx];
const newNotifications = { ...notifications };
delete newNotifications[notification.index];

return (
<Alert
variant={AlertVariant[notification.variant]}
title={notification.title}
actionClose={
<AlertActionCloseButton
title={notifications.title}
onClose={() => setNotifications(newNotifications)}
/>
}
key={notification.index}>
{notification.message}
</Alert>
);
})}
</AlertGroup>}
<AddressContext.Provider value={address}>
<WithDialogs>
<AnacondaWizard
isBootIso={isBootIso}
onCritFail={onCritFail}
onAddErrorNotification={onAddErrorNotification}
title={title}
storageData={state.storage}
localizationData={state.localization}
dispatch={dispatch}
conf={conf}
osRelease={osRelease}
/>
</WithDialogs>
</AddressContext.Provider>
</Page>
<>
{criticalError &&
<CriticalError exception={criticalError} isBootIso={isBootIso} reportLinkURL={bzReportURL} />}
<Page
data-debug={conf.Anaconda.debug}
additionalGroupedContent={
<AnacondaHeader beta={beta} title={title} />
}
groupProps={{
sticky: "top"
}}
>
{Object.keys(notifications).length > 0 &&
<AlertGroup isToast isLiveRegion>
{Object.keys(notifications).map(idx => {
const notification = notifications[idx];
const newNotifications = { ...notifications };
delete newNotifications[notification.index];

return (
<Alert
variant={AlertVariant[notification.variant]}
title={notification.title}
actionClose={
<AlertActionCloseButton
title={notifications.title}
onClose={() => setNotifications(newNotifications)}
/>
}
key={notification.index}>
{notification.message}
</Alert>
);
})}
</AlertGroup>}
<AddressContext.Provider value={address}>
<WithDialogs>
<AnacondaWizard
isBootIso={isBootIso}
onCritFail={onCritFail}
onAddErrorNotification={onAddErrorNotification}
title={title}
storageData={state.storage}
localizationData={state.localization}
dispatch={dispatch}
conf={conf}
osRelease={osRelease}
/>
</WithDialogs>
</AddressContext.Provider>
</Page>
)
</>
);

return (
Expand Down
3 changes: 2 additions & 1 deletion ui/webui/src/components/installation/InstallationProgress.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { exitGui } from "../../helpers/exit.js";
import "./InstallationProgress.scss";

const _ = cockpit.gettext;
const N_ = cockpit.noop;

export const InstallationProgress = ({ onCritFail, idPrefix, isBootIso }) => {
const [status, setStatus] = useState();
Expand Down Expand Up @@ -89,7 +90,7 @@ export const InstallationProgress = ({ onCritFail, idPrefix, isBootIso }) => {
});
taskProxy.addEventListener("Stopped", () => {
taskProxy.Finish().catch(onCritFail({
context: cockpit.format(_("Installation of the system failed: $0"), refStatusMessage.current),
context: cockpit.format(N_("Installation of the system failed: $0"), refStatusMessage.current),
}));
});
taskProxy.addEventListener("Succeeded", () => {
Expand Down
3 changes: 2 additions & 1 deletion ui/webui/src/components/storage/InstallationMethod.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { debug } from "../../helpers/log.js";
import "./InstallationMethod.scss";

const _ = cockpit.gettext;
const N_ = cockpit.noop;

/**
* Select default disks for the partitioning.
Expand Down Expand Up @@ -125,7 +126,7 @@ const InstallationDestination = ({ deviceData, diskSelection, dispatch, idPrefix
const loading = !deviceData || diskSelection.usableDisks.some(disk => !deviceData[disk]);

const errorHandler = onCritFail({
context: _("Rescanning of the disks failed.")
context: N_("Rescanning of the disks failed.")
});

const rescanDisksButton = (
Expand Down