Handle error scenarios (#155)

* Handle error scenarios for all http requests * Track errors and exceptions to AppInsights
2019-04-29 11:23:58 -07:00 · 2019-04-29 11:23:58 -07:00 · c340384d85
--- a/ServerlessLibraryAPI/ClientApp/src/App.js
+++ b/ServerlessLibraryAPI/ClientApp/src/App.js
@ -1,6 +1,7 @@
 import React, { Component } from "react";
 import { Switch, Route, withRouter } from "react-router-dom";
 import { connect } from "react-redux";
+import { Dialog } from "office-ui-fabric-react";

 import "./App.css";

@ -12,21 +13,48 @@ import { sampleActions } from "./actions/sampleActions";
 import { userActions } from "./actions/userActions";
 import { libraryService, userService } from "./services";

+const loginErrorMsg = "We were unable to log you in. Please try again later.";
+
 class App extends Component {
+  constructor(props) {
+    super(props);
+
+    this.state = {
+      showErrorDialog: false
+    };
+    this.onDismissErrorDialog = this.onDismissErrorDialog.bind(this);
+  }
+
  componentDidMount() {
    libraryService
      .getAllSamples()
      .then(samples => this.props.getSamplesSuccess(samples))
-      .catch(error => console.log(error));
+      .catch(() => {
+        // do nothing
+      });

    this.props.getCurrentUserRequest();
    userService
      .getCurrentUser()
      .then(user => this.props.getCurrentUserSuccess(user))
-      .catch(error => this.props.getCurrentUserFailure());
+      .catch(data => {
+        this.props.getCurrentUserFailure();
+        if (data.status !== 401) {
+          this.setState({
+            showErrorDialog: true
+          });
+        }
+      });
+  }
+
+  onDismissErrorDialog() {
+    this.setState({
+      showErrorDialog: false
+    });
  }

  render() {
+    const { showErrorDialog } = this.state;
    return (
      <div id="container">
        <div id="header">
@ -39,6 +67,17 @@ class App extends Component {
            <Route exact path="/contribute" component={ContributionsPage} />
          </Switch>
        </div>
+        {showErrorDialog && (
+          <Dialog
+            dialogContentProps={{
+              title: "An error occurred!"
+            }}
+            hidden={!showErrorDialog}
+            onDismiss={this.onDismissErrorDialog}
+          >
+            <p>{loginErrorMsg}</p>
+          </Dialog>
+        )}
      </div>
    );
  }
--- a/ServerlessLibraryAPI/ClientApp/src/components/Contribute/AddContributionForm.js
+++ b/ServerlessLibraryAPI/ClientApp/src/components/Contribute/AddContributionForm.js
@ -124,15 +124,16 @@ class AddContributionForm extends Component {
      this.setErrorState(errors);
      return;
    }
-    libraryService.submitNewSample(sample).then(
-      sample => {
+    libraryService
+      .submitNewSample(sample)
+      .then(sample => {
+        console.log(sample); // todo - give a notification to the user
        this.props.sampleSubmittedSuccess(sample);
        this.resetForm();
-      },
-      error => {
-        this.setErrorState(error);
-      }
-    );
+      })
+      .catch(data => {
+        this.setErrorState(data.error);
+      });
  }

  onDismissErrorDialog() {
@ -176,11 +177,15 @@ class AddContributionForm extends Component {
                hidden={!showErrorDialog}
                onDismiss={this.onDismissErrorDialog}
              >
-                <ul>
-                  {errors.map((message, index) => (
-                    <li key={index}>{message}</li>
-                  ))}
-                </ul>
+                {Array.isArray(errors) ? (
+                  <ul>
+                    {errors.map((message, index) => (
+                      <li key={index}>{message}</li>
+                    ))}
+                  </ul>
+                ) : (
+                  <p>{String(errors)}</p>
+                )}
              </Dialog>
            )}
            <div className="input-container">
--- a/ServerlessLibraryAPI/ClientApp/src/components/DetailView/ActionBar.js
+++ b/ServerlessLibraryAPI/ClientApp/src/components/DetailView/ActionBar.js
@ -14,10 +14,21 @@ class ActionBar extends Component {
  }

  outboundDeployClick() {
-    libraryService.updateDownloadCount(this.props.id);
+    this.updateDownloadCount(this.props.id);
    this.trackUserActionEvent("/sample/deploy/agree");
  }

+  updateDownloadCount(id) {
+    libraryService
+      .updateDownloadCount(id)
+      .then(() => {
+        // do nothing
+      })
+      .catch(() => {
+        // do nothing
+      });
+  }
+
  getDeployLink(template) {
    return (
      "https://portal.azure.com/#create/Microsoft.Template/uri/" +
@ -34,7 +45,7 @@ class ActionBar extends Component {
  }

  openInVSCodeClick() {
-    libraryService.updateDownloadCount(this.props.id);
+    this.updateDownloadCount(this.props.id);
    this.trackUserActionEvent("/sample/openinvscode");
  }

@ -49,18 +60,14 @@ class ActionBar extends Component {
  }

  render() {
-    let { repository, template } = this.props;
-    let deployDisabled = false;
-    if (!template) {
-      deployDisabled = true;
-    }
+    const { repository, template } = this.props;

    return (
      <div className="action-container">
        <div className="action-item">
          <FabricLink
            href={this.getDeployLink(template)}
-            disabled={deployDisabled}
+            disabled={!template}
            target="_blank"
            onClick={this.outboundDeployClick}
          >
@ -73,6 +80,7 @@ class ActionBar extends Component {
        <div className="action-item">
          <FabricLink
            href={this.getOpenInVSCodeLink()}
+            disabled={!repository}
            onClick={this.openInVSCodeClick}
          >
            <div className="action-link-wrapper">
@ -84,6 +92,7 @@ class ActionBar extends Component {
        <div className="action-item">
          <FabricLink
            href={repository}
+            disabled={!repository}
            target="_blank"
            onClick={this.outboundRepoClick}
          >
--- a/ServerlessLibraryAPI/ClientApp/src/components/DetailView/DetailPageContent.js
+++ b/ServerlessLibraryAPI/ClientApp/src/components/DetailView/DetailPageContent.js
@ -6,45 +6,34 @@ import {
  PivotLinkSize,
  ScrollablePane
 } from "office-ui-fabric-react/lib/index";
+import { githubService } from "../../services";
 import "./DetailPageContent.scss";

+const defaultLicenseText =
+  "Each application is licensed to you by its owner (which may or may not be Microsoft) under the agreement which accompanies the application. Microsoft is not responsible for any non-Microsoft code and does not screen for security, compatibility, or performance. The applications are not supported by any Microsoft support program or service. The applications are provided AS IS without warranty of any kind.";
+
 class DetailPageContent extends Component {
  constructor(props) {
    super(props);
    this.state = {
      armTemplateText: "",
      markdownText: "",
+      licenseText: "",
      selectedKey: "overview"
    };

    this.handleLinkClick = this.handleLinkClick.bind(this);
-   }
+  }

-   // This method is used to fetch readme content from repo when valid repository url is received as props
+  // This method is used to fetch readme content from repo when valid repository url is received as props
  componentDidUpdate(prevProps, prevState) {
    if (
      this.props.repository !== prevProps.repository &&
      prevState.markdownText === ""
    ) {
      let { repository } = this.props;
-      repository = repository.replace(
-        "https://github.com",
-        "https://raw.githubusercontent.com"
-      );
-
-      repository = repository.replace("/tree/", "/");
-      let readmefileUrl = repository + "/master/README.md";
-      if (repository.includes("/master/")) {
-        readmefileUrl = repository + "/README.md";
-      }
-
-      fetch(readmefileUrl)
-        .then(response => {
-          if (response.ok) {
-            return response.text();
-          }
-          throw new Error("Network response was not ok.");
-        })
+      githubService
+        .getReadMe(repository)
        .then(data => {
          var r = new RegExp(
            "https?://Azuredeploy.net/deploybutton.(png|svg)",
@ -53,41 +42,43 @@ class DetailPageContent extends Component {
          data = data.replace(r, "");
          this.setState({ markdownText: data });
        })
-        .catch(error =>
-          this.setState({ markdownText: "No readme file available " })
-        );
+        .catch(() => {
+          // do nothing
+        });
    }
  }

  handleLinkClick(pivotItem, ev) {
-    this.setState({
-      selectedKey: pivotItem.props.itemKey
-    });
-    if (
-      pivotItem.props.itemKey === "armtemplate" &&
-      this.state.armTemplateText === ""
-    ) {
-      let { template } = this.props;
+    const selectedKey = pivotItem.props.itemKey;
+    this.setState({ selectedKey });
+    if (selectedKey === "armtemplate" && this.state.armTemplateText === "") {
+      const { template } = this.props;
      if (template) {
-        fetch(template)
-          .then(response => {
-            if (response.ok) {
-              return response.text();
-            }
-            throw new Error("Network response was not ok.");
-          })
-          .then(data => {
-            this.setState({ armTemplateText: data });
-          })
-          .catch(error =>
-            this.setState({ armTemplateText: "Unable to fetch ARM template." })
-          );
-      } else {
-        this.setState({
-          armTemplateText: "This sample does not have an arm template."
-        });
+        githubService
+          .getArmTemplate(template)
+          .then(data =>
+            this.setState({
+              armTemplateText: data
+            })
+          )
+          .catch(() => {
+            // do nothing
+          });
      }
    }
+    if (selectedKey === "license" && this.state.licenseText === "") {
+      const { license, repository } = this.props;
+      githubService
+        .getLicense(license, repository)
+        .then(data =>
+          this.setState({
+            licenseText: data
+          })
+        )
+        .catch(() => {
+          // do nothing
+        });
+    }
  }

  render() {
@ -101,11 +92,17 @@ class DetailPageContent extends Component {
      }
    };

+    const {
+      selectedKey,
+      markdownText,
+      licenseText,
+      armTemplateText
+    } = this.state;
    return (
      <div className="detail-page-content">
        <Pivot
          styles={pivotStyles}
-          selectedKey={this.state.selectedKey}
+          selectedKey={selectedKey}
          linkSize={PivotLinkSize.large}
          onLinkClick={(item, ev) => this.handleLinkClick(item, ev)}
        >
@ -113,7 +110,7 @@ class DetailPageContent extends Component {
            <div className="pivot-item-container">
              <div className="scrollablePane-wrapper">
                <ScrollablePane>
-                  <ReactMarkdown>{this.state.markdownText}</ReactMarkdown>
+                  <ReactMarkdown>{markdownText}</ReactMarkdown>
                </ScrollablePane>
              </div>
            </div>
@ -123,22 +120,10 @@ class DetailPageContent extends Component {
              <div className="scrollablePane-wrapper">
                <ScrollablePane>
                  <div className="license-content">
-                    <p>
-                      Each application is licensed to you by its owner (which
-                      may or may not be Microsoft) under the agreement which
-                      accompanies the application. Microsoft is not responsible
-                      for any non-Microsoft code and does not screen for
-                      security, compatibility, or performance. The applications
-                      are not supported by any Microsoft support program or
-                      service. The applications are provided AS IS without
-                      warranty of any kind
-                    </p>
-                    <p>
-                      Also, please note that the Function App you've selected
-                      was created with Azure Functions 1.x. As such, it might
-                      not contain the latest features, but will still work as
-                      provided.
-                    </p>
+                    <p>{defaultLicenseText}</p>
+                    {licenseText !== "" && (
+                      <p style={{ borderTop: "2px outset" }}>{licenseText}</p>
+                    )}
                  </div>
                </ScrollablePane>
              </div>
@ -149,7 +134,7 @@ class DetailPageContent extends Component {
              <div className="scrollablePane-wrapper">
                <ScrollablePane>
                  <div className="armtemplate-content">
-                    <pre>{this.state.armTemplateText}</pre>
+                    <pre>{armTemplateText}</pre>
                  </div>
                </ScrollablePane>
              </div>
--- a/ServerlessLibraryAPI/ClientApp/src/components/DetailView/DetailPageContent.scss
+++ b/ServerlessLibraryAPI/ClientApp/src/components/DetailView/DetailPageContent.scss
@ -1,4 +1,4 @@
-.detail-page-content{
+.detail-page-content {
  margin-top: 24px;
  border-top: 1px solid rgba(105, 130, 155, 0.25);
  border-bottom: 1px solid rgba(105, 130, 155, 0.25);
@ -22,5 +22,5 @@
 .license-content {
  max-width: 750px;
  text-align: justify;
-  text-justify: inter-word;
+  white-space: pre-line;
 }
--- a/ServerlessLibraryAPI/ClientApp/src/components/DetailView/DetailView.js
+++ b/ServerlessLibraryAPI/ClientApp/src/components/DetailView/DetailView.js
@ -1,6 +1,5 @@
 import React, { Component } from "react";
 import { connect } from "react-redux";
-import { libraryService } from "../../services";
 import ActionBar from "./ActionBar";
 import DetailPageContent from "./DetailPageContent";
 import DetailPageHeader from "./DetailPageHeader";
@ -16,20 +15,18 @@ class DetailView extends Component {
  }

  componentDidMount() {
-    var id = this.props.match.params.id;
-    var currentItem;
-    if (this.props.samples.length > 0) {
-      currentItem = this.props.samples.filter(s => s.id === id)[0];
+    this.setCurrentItemInState();
+  }
+
+  componentDidUpdate(prevProps, prevState) {
+    this.setCurrentItemInState();
+  }
+
+  setCurrentItemInState() {
+    if (!this.state.sample.id && this.props.samples.length > 0) {
+      const id = this.props.match.params.id;
+      let currentItem = this.props.samples.filter(s => s.id === id)[0] || {};
      this.setState({ sample: currentItem });
-    } else {
-      libraryService
-        .getAllSamples()
-        .then(samples => {
-          this.props.getSamplesSuccess(samples);
-          currentItem = samples.filter(s => s.id === id)[0];
-          this.setState({ sample: currentItem });
-        })
-        .catch(error => console.log(error));
    }
  }

--- a/ServerlessLibraryAPI/ClientApp/src/components/Header/AuthControl.js
+++ b/ServerlessLibraryAPI/ClientApp/src/components/Header/AuthControl.js
@ -72,7 +72,14 @@ class AuthControl extends Component {

  _onSignoutClick() {
    this.props.logout(); // clear the redux store before making a call to the backend
-    userService.logout();
+    userService
+      .logout()
+      .then(() => {
+        // do nothing
+      })
+      .catch(() => {
+        // do nothing
+      });
  }

  render() {
--- a/ServerlessLibraryAPI/ClientApp/src/components/MetricBar/MetricBar.js
+++ b/ServerlessLibraryAPI/ClientApp/src/components/MetricBar/MetricBar.js
@ -20,7 +20,6 @@ class MetricBar extends Component {
  }

  handleLikeClick() {
-
    // If user already liked, then decrement like count and set the sentiment state to none.
    // If in past disliked and choose to like the sample, then decrement dislike count and increment like count
    // If not action taken ealier, just increment like count and set sentiment state to liked.
@ -47,28 +46,32 @@ class MetricBar extends Component {
    localStorage.setItem(this.props.id, choice);
    this.setState({ sentimentAction: choice });

-    var sentimentPayload={
-      Id:this.props.id,
-      LikeChanges:likeChanges,
-      DislikeChanges:dislikeChanges
-    }
+    var sentimentPayload = {
+      Id: this.props.id,
+      LikeChanges: likeChanges,
+      DislikeChanges: dislikeChanges
+    };

    libraryService
      .updateUserSentimentStats(sentimentPayload)
-      .then(response => response.body)
-      .catch(error => console.log(error));
+      .then(() => {
+        // do nothing
+      })
+      .catch(() => {
+        // do nothing
+      });
  }

  render() {
-    let {author, downloads, createddate, likes, dislikes} = this.props;
-    let createdonDate = new  Date(createddate);
+    let { author, downloads, createddate, likes, dislikes } = this.props;
+    let createdonDate = new Date(createddate);
    let createdonLocaleDate = createdonDate.toLocaleDateString();

    let likeIconName = "Like";
    let likeTitle = "Like";
    let dislikeIconName = "Dislike";
    let dislikeTitle = "Dislike";
- 
+
    if (this.state.sentimentAction === "liked") {
      likeIconName = "LikeSolid";
      likeTitle = "Liked";
@ -80,7 +83,7 @@ class MetricBar extends Component {
      dislikeTitle = "Disliked";
      dislikes = dislikes + 1;
    }
-   
+
    const styles = {
      button: {
        width: 16,
@ -94,7 +97,8 @@ class MetricBar extends Component {
      <div className="metrics">
        <div>
          <span>
-            By: {author} | {downloads} downloads | Created on: {createdonLocaleDate} |
+            By: {author} | {downloads} downloads | Created on:{" "}
+            {createdonLocaleDate} |
          </span>
        </div>

--- a/ServerlessLibraryAPI/ClientApp/src/helpers/appinsights.js
+++ b/ServerlessLibraryAPI/ClientApp/src/helpers/appinsights.js
@ -4,3 +4,17 @@ export function trackEvent(eventName, eventData) {
    appInsights.trackEvent(eventName, eventData);
  }
 }
+
+export function trackError(errorString, properties) {
+  let appInsights = window.appInsights;
+  if (typeof appInsights !== "undefined") {
+    appInsights.trackTrace(errorString, properties, 3);
+  }
+}
+
+export function trackException(exception, properties) {
+  let appInsights = window.appInsights;
+  if (typeof appInsights !== "undefined") {
+    appInsights.trackException(exception, null, properties);
+  }
+}
--- a/ServerlessLibraryAPI/ClientApp/src/helpers/handle-response.js
+++ b/ServerlessLibraryAPI/ClientApp/src/helpers/handle-response.js
@ -1,11 +1,37 @@
-export function handleResponse(response) {
-  return response.text().then(text => {
-    if (!response.ok) {
-      const error = text || response.statusText;
-      return Promise.reject(error);
-    }
+import { trackError, trackException } from "./appinsights";

-    const json = text && JSON.parse(text);
-    return json;
+export function handleResponse(response) {
+  try {
+    return response.text().then(text => {
+      if (response.ok) {
+        return text;
+      }
+      const error = {
+        status: response.status,
+        error: text || response.statusText
+      };
+      trackError(error.error, { ...error, url: response.url });
+      return Promise.reject(error);
+    });
+  } catch (ex) {
+    trackException(ex, { url: response.url, method: "handleResponse" });
+    return Promise.reject({
+      status: -1,
+      error: "Encountered unexpected exception."
+    });
+  }
+}
+
+export function handleJsonResponse(response) {
+  return handleResponse(response).then(data => {
+    try {
+      return JSON.parse(data);
+    } catch (ex) {
+      trackException(ex, { url: response.url, method: "handleJsonResponse" });
+      return Promise.reject({
+        status: -1,
+        error: "Encountered unexpected exception."
+      });
+    }
  });
 }
--- a/ServerlessLibraryAPI/ClientApp/src/services/github.service.js
+++ b/ServerlessLibraryAPI/ClientApp/src/services/github.service.js
@ -0,0 +1,39 @@
+import { handleResponse } from "../helpers";
+
+export const githubService = {
+  getReadMe,
+  getLicense,
+  getArmTemplate
+};
+
+function getRawContentUrl(repoUrl, fileName) {
+  let rawUrl = repoUrl
+    .replace("https://github.com", "https://raw.githubusercontent.com")
+    .replace("/tree/", "/");
+  rawUrl = rawUrl.includes("/master/") ? rawUrl + "/" : rawUrl + "/master/";
+  let contentUrl = rawUrl + fileName;
+  return contentUrl;
+}
+
+function getReadMe(repoUrl) {
+  const requestOptions = {
+    method: "GET"
+  };
+  const readMeUrl = getRawContentUrl(repoUrl, "README.md");
+  return fetch(readMeUrl, requestOptions).then(handleResponse);
+}
+
+function getLicense(licenseUrl, repoUrl) {
+  const requestOptions = {
+    method: "GET"
+  };
+  const contentUrl = licenseUrl || getRawContentUrl(repoUrl, "LICENSE");
+  return fetch(contentUrl, requestOptions).then(handleResponse);
+}
+
+function getArmTemplate(templateUrl) {
+  const requestOptions = {
+    method: "GET"
+  };
+  return fetch(templateUrl, requestOptions).then(handleResponse);
+}
--- a/ServerlessLibraryAPI/ClientApp/src/services/index.js
+++ b/ServerlessLibraryAPI/ClientApp/src/services/index.js
@ -1,4 +1,3 @@
 export * from "./user.service";
 export * from "./library.service";
-
-export const useMockApi = false;
+export * from "./github.service";
--- a/ServerlessLibraryAPI/ClientApp/src/services/library.service.js
+++ b/ServerlessLibraryAPI/ClientApp/src/services/library.service.js
@ -1,5 +1,5 @@
-import { handleResponse } from "../helpers";
-import { useMockApi } from "./index";
+import { handleResponse, handleJsonResponse } from "../helpers";
+import { trackException } from "../helpers/appinsights";

 export const libraryService = {
  getAllSamples,
@ -13,7 +13,7 @@ function getAllSamples() {
    method: "GET"
  };

-  return fetch("/api/Library", requestOptions).then(handleResponse);
+  return fetch("/api/Library", requestOptions).then(handleJsonResponse);
 }

 function submitNewSample(item) {
@ -24,19 +24,22 @@ function submitNewSample(item) {
      "Content-Type": "application/json"
    }
  };
-  return fetch("/api/library", requestOptions).then(response => {
-    if (response.ok) {
-      return response.json();
-    }
-
-    if (response.status === 400) {
-      return response.json().then(json => Promise.reject(json));
-    }
-
-    return response
-      .text()
-      .then(text => Promise.reject(text || response.statusText));
-  });
+  return fetch("/api/library", requestOptions)
+    .then(handleJsonResponse)
+    .catch(data => {
+      let error = data.error;
+      if (data.status === 400) {
+        try {
+          error = JSON.parse(data.error);
+        } catch (ex) {
+          trackException(ex, { method: "submitNewSample" });
+        }
+      }
+      return Promise.reject({
+        status: data.status,
+        error: error
+      });
+    });
 }

 function updateUserSentimentStats(sentimentPayload) {
@ -58,5 +61,5 @@ function updateDownloadCount(id) {
      "Content-Type": "application/json"
    }
  };
-  return fetch("/api/metrics", requestOptions).then(handleResponse);
+  return fetch("/api/metrics/downloads", requestOptions).then(handleResponse);
 }
--- a/ServerlessLibraryAPI/ClientApp/src/services/user.service.js
+++ b/ServerlessLibraryAPI/ClientApp/src/services/user.service.js
@ -1,44 +1,18 @@
-import { handleResponse } from "../helpers";
-import { useMockApi } from "./index";
+import { handleResponse, handleJsonResponse } from "../helpers";

 export const userService = {
  getCurrentUser,
  logout
 };

-const validUser = {
-  displayName: "Neha",
-  fullName: "Neha Gupta",
-  email: "abc@xyz.com",
-  avatarUrl: "https://avatars2.githubusercontent.com/u/45184761?v=4",
-  userName: "msnehagup"
-};
-
-// const invalidUser = {
-//   abc: 'xyz'
-// };
-
-function getMockUser() {
-  return Promise.resolve(validUser);
-  // return Promise.reject("No User is signed in!!");
-}
-
 function getCurrentUser() {
-  if (useMockApi) {
-    return getMockUser();
-  }
-
  const requestOptions = {
    method: "GET"
  };
-  return fetch("/api/user", requestOptions).then(handleResponse);
+  return fetch("/api/user", requestOptions).then(handleJsonResponse);
 }

 function logout() {
-  if (useMockApi) {
-    return Promise.resolve();
-  }
-
  const requestOptions = {
    method: "GET"
  };
--- a/ServerlessLibraryAPI/Controllers/MetricsController.cs
+++ b/ServerlessLibraryAPI/Controllers/MetricsController.cs
@ -4,14 +4,14 @@ using ServerlessLibrary.Models;

 namespace ServerlessLibrary.Controllers
 {
-    [Route("api/[controller]")]
+    [Route("api/[controller]/[action]")]
    [ApiController]
    public class MetricsController : ControllerBase
    {
-        // PUT api/<controller>
+        // PUT api/<controller>/downloads
        [ProducesResponseType(typeof(bool), 200)]
        [HttpPut]
-        public JsonResult Put([FromBody]string id)
+        public JsonResult Downloads([FromBody]string id)
        {
            StorageHelper.updateUserStats(JsonConvert.SerializeObject(new { id, userAction = "download" }));
            return new JsonResult(true);
@ -19,7 +19,6 @@ namespace ServerlessLibrary.Controllers
        // PUT api/<controller>/sentiment
        [ProducesResponseType(typeof(bool), 200)]
        [HttpPut]
-        [Route("sentiment")]
        public IActionResult Sentiment([FromBody]SentimentPayload sentimentPayload)
        {
            if (sentimentPayload.LikeChanges < -1