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.

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

Show error message for unexpected errors #866

Merged
merged 2 commits into from
Dec 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions dashboard/src/components/ErrorAlert/ErrorSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,9 @@ describe("Default error", () => {
const wrapper = shallow(<ErrorSelector error={new Error("surprise!")} resource="my-app" />);
expect(wrapper.html()).toContain(shallow(genericMessage).html());
});

it("Should contain the message of the error", () => {
const wrapper = shallow(<ErrorSelector error={new Error("surprise!")} resource="my-app" />);
expect(wrapper.html()).toContain("surprise!");
});
});
2 changes: 1 addition & 1 deletion dashboard/src/components/ErrorAlert/ErrorSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ const ErrorSelector: React.SFC<IErrorSelectorProps> = props => {
/>
);
default:
return <UnexpectedErrorAlert />;
return <UnexpectedErrorAlert raw={true} text={error.message} showGenericMessage={true} />;
}
};

Expand Down
16 changes: 3 additions & 13 deletions dashboard/src/components/ErrorAlert/UnexpectedErrorAlert.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@ import * as React from "react";
import ErrorPageHeader from "./ErrorAlertHeader";
import UnexpectedErrorAlert, { genericMessage } from "./UnexpectedErrorAlert";

describe("when no text is passed", () => {
it("renders a default troubleshooting info", () => {
const wrapper = shallow(<UnexpectedErrorAlert />);
expect(wrapper.text()).toContain(shallow(genericMessage).text());
expect(wrapper).toMatchSnapshot();
});
});

describe("when text is passed", () => {
const defaultProps = {
text: "This is my error message",
Expand Down Expand Up @@ -68,7 +60,7 @@ describe("icon", () => {

describe("genericMessage", () => {
it("renders the default message", () => {
const wrapper = shallow(<UnexpectedErrorAlert />);
const wrapper = shallow(<UnexpectedErrorAlert showGenericMessage={true} />);
expect(wrapper.html()).toContain(shallow(genericMessage).html());
});
it("avoids the default message", () => {
Expand All @@ -84,13 +76,11 @@ describe("children", () => {
<div>more info!</div>
</UnexpectedErrorAlert>,
);
// It should contain two elements error__content, one for the default message and one for the children
expect(wrapper.find(".error__content").length).toBe(2);
expect(wrapper.find(".error__content").length).toBe(1);
expect(wrapper.html()).toContain("more info!");
});
it("avoids extra elements", () => {
const wrapper = shallow(<UnexpectedErrorAlert />);
// It should contain only one error__content: the default message
expect(wrapper.find(".error__content").length).toBe(1);
expect(wrapper.find(".error__content").length).toBe(0);
});
});
18 changes: 11 additions & 7 deletions dashboard/src/components/ErrorAlert/UnexpectedErrorAlert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const genericMessage = (
<p>Troubleshooting:</p>
<ul className="error__troubleshooting">
<li>Check for network issues.</li>
<li>Check your browser's JavaScript console for errors.</li>
<li>Check your browser's JavaScript console for additional errors.</li>
<li>
Check the health of Kubeapps components{" "}
<code>helm status &lt;kubeapps_release_name&gt;</code>.
Expand All @@ -35,20 +35,21 @@ export const genericMessage = (
class UnexpectedErrorPage extends React.Component<IUnexpectedErrorPage> {
public static defaultProps: Partial<IUnexpectedErrorPage> = {
title: "Sorry! Something went wrong.",
showGenericMessage: false,
};
public render() {
let message = genericMessage;
let customMessage = null;
if (this.props.text) {
if (this.props.raw) {
message = (
<div className="error__content margin-l-enormous">
customMessage = (
<div className="error__content">
<section className="Terminal terminal__error elevation-1 type-color-white">
{this.props.text}
</section>
</div>
);
} else {
message = <p>{this.props.text}</p>;
customMessage = <p>{this.props.text}</p>;
}
}
// NOTE(miguel) We are using the non-undefined "!" token in `props.title` because our current version of
Expand All @@ -57,8 +58,11 @@ class UnexpectedErrorPage extends React.Component<IUnexpectedErrorPage> {
return (
<div className="alert alert-error margin-v-bigger">
<ErrorPageHeader icon={this.props.icon || X}>{this.props.title!}</ErrorPageHeader>
{(this.props.showGenericMessage !== false || message !== genericMessage) && (
<div className="error__content margin-l-enormous">{message}</div>
{customMessage && (
<div className="error__content margin-l-enormous margin-b-big">{customMessage}</div>
)}
{this.props.showGenericMessage && (
<div className="error__content margin-l-enormous">{genericMessage}</div>
)}
{this.props.children && (
<div className="error__content margin-l-enormous">{this.props.children}</div>
Expand Down

This file was deleted.

15 changes: 14 additions & 1 deletion dashboard/src/shared/AxiosInstance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe("createAxiosInterceptor", () => {

it("returns the generic error message otherwise", async () => {
moxios.stubRequest(testPath, {
response: { message: "this will be ignored" },
response: {},
status: 555,
});

Expand All @@ -108,6 +108,19 @@ describe("createAxiosInterceptor", () => {
}
});

it("returns the response message", async () => {
moxios.stubRequest(testPath, {
response: { message: "this is an error!" },
status: 555,
});

try {
await axios.get(testPath);
} catch (error) {
expect(error.message).toBe("this is an error!");
}
});

it("dispatches auth error and logout if 401", async () => {
const expectedActions = [
{
Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/shared/AxiosInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export function createAxiosInterceptors(axios: AxiosInstance, store: Store<IStor
case 422:
return Promise.reject(new UnprocessableEntity(message));
default:
return Promise.reject(e);
return Promise.reject(new Error(message));
}
},
);
Expand Down