Skip to content

Commit

Permalink
Merge pull request #411 from satoshipay/feature/298-less-intrusive-st…
Browse files Browse the repository at this point in the history
…ream-errors

Less stream error noise when offline
  • Loading branch information
andywer authored Feb 22, 2019
2 parents ea997e7 + 1fb7509 commit ee3362b
Show file tree
Hide file tree
Showing 12 changed files with 98 additions and 45 deletions.
10 changes: 5 additions & 5 deletions src/components/Dialog/ChangePassword.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ interface Props {
}

function ChangePasswordDialog(props: Props) {
const { addError, addNotification } = useContext(NotificationsContext)
const { showError, showNotification } = useContext(NotificationsContext)
const [errors, setErrors] = useState<Errors>({})
const [formValues, setFormValues] = useState<FormValues>({
nextPassword: "",
Expand All @@ -116,10 +116,10 @@ function ChangePasswordDialog(props: Props) {
props
.changePassword(accountID, prevPassword, nextPassword)
.then(() => {
addNotification("success", requiresPassword ? "Password changed." : "Password set.")
showNotification("success", requiresPassword ? "Password changed." : "Password set.")
props.onClose()
})
.catch(addError)
.catch(showError)
}
}
const onClose = () => {
Expand All @@ -139,10 +139,10 @@ function ChangePasswordDialog(props: Props) {
props
.removePassword(props.account.id, formValues.prevPassword)
.then(() => {
addNotification("success", "Password removed.")
showNotification("success", "Password removed.")
props.onClose()
})
.catch(addError)
.catch(showError)
}
}
const setFormValue = (name: keyof FormValues, value: string) => {
Expand Down
4 changes: 2 additions & 2 deletions src/components/Dialog/ReceivePayment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ interface Props {
}

function ReceivePaymentDialog(props: Props) {
const { addNotification } = useContext(NotificationsContext)
const { showNotification } = useContext(NotificationsContext)

const copyToClipboard = async () => {
await (navigator as any).clipboard.writeText(props.account.publicKey)
addNotification("info", "Copied to clipboard.")
showNotification("info", "Copied to clipboard.")
}
return (
<Dialog open={props.open} fullScreen onClose={props.onClose} TransitionComponent={Transition}>
Expand Down
39 changes: 30 additions & 9 deletions src/components/NotificationContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from "react"
import { useContext, useRef, useState } from "react"
import Snackbar from "@material-ui/core/Snackbar"
import Snackbar, { SnackbarOrigin } from "@material-ui/core/Snackbar"
import SnackbarContent from "@material-ui/core/SnackbarContent"
import CheckIcon from "@material-ui/icons/CheckCircle"
import ErrorIcon from "@material-ui/icons/Error"
Expand All @@ -9,6 +9,7 @@ import blue from "@material-ui/core/colors/blue"
import green from "@material-ui/core/colors/green"
import withStyles, { ClassNameMap, StyleRulesCallback } from "@material-ui/core/styles/withStyles"
import { Notification, NotificationsContext, NotificationType } from "../context/notifications"
import { useOnlineStatus } from "../hooks"

const icons: { [key in NotificationType]: React.ComponentType<any> } = {
error: ErrorIcon,
Expand Down Expand Up @@ -42,13 +43,16 @@ const styles: StyleRulesCallback = theme => ({
})

interface NotificationProps {
anchorOrigin?: SnackbarOrigin
autoHideDuration?: number
classes: ClassNameMap<keyof ReturnType<typeof styles>>
contentStyle?: React.CSSProperties
message: string
type: NotificationType
open?: boolean
onClick?: () => void
onClose?: () => void
style?: React.CSSProperties
}

function NotificationSnackbar(props: NotificationProps) {
Expand All @@ -63,11 +67,13 @@ function NotificationSnackbar(props: NotificationProps) {

return (
<Snackbar
anchorOrigin={props.anchorOrigin}
autoHideDuration={props.autoHideDuration}
className={props.onClick ? classes.clickable : undefined}
open={open}
onClick={props.onClick}
onClose={props.onClose}
style={props.style}
>
<SnackbarContent
className={contentClassnames[props.type]}
Expand All @@ -77,6 +83,7 @@ function NotificationSnackbar(props: NotificationProps) {
{props.message}
</span>
}
style={props.contentStyle}
/>
</Snackbar>
)
Expand All @@ -86,6 +93,7 @@ const StyledNotification = withStyles(styles)(NotificationSnackbar)

function NotificationsContainer() {
const { notifications } = useContext(NotificationsContext)
const { isOnline } = useOnlineStatus()
const [lastClosedNotificationID, setLastClosedNotificationID] = useState(0)
const lastShownNotification = useRef<Notification | null>(null)

Expand All @@ -103,14 +111,27 @@ function NotificationsContainer() {
}

return (
<StyledNotification
autoHideDuration={5000}
message={notification ? notification.message : ""}
type={notification ? notification.type : "error"}
open={open}
onClick={notification ? notification.onClick : undefined}
onClose={() => closeNotification(notification)}
/>
<>
<StyledNotification
autoHideDuration={5000}
message={notification ? notification.message : ""}
type={notification ? notification.type : "error"}
open={open}
onClick={notification ? notification.onClick : undefined}
onClose={() => closeNotification(notification)}
/>
<StyledNotification
anchorOrigin={{
horizontal: "left",
vertical: "bottom"
}}
contentStyle={{ minWidth: 0 }}
message="Offline"
open={!isOnline}
style={{ bottom: 0 }}
type="error"
/>
</>
)
}

Expand Down
20 changes: 10 additions & 10 deletions src/context/notifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ interface NotificationOptions {

interface ContextValue {
notifications: Notification[]
addError(error: any): void
addNotification(type: NotificationType, message: string, props?: NotificationOptions): void
showError(error: any): void
showNotification(type: NotificationType, message: string, props?: NotificationOptions): void
}

interface Props {
Expand All @@ -33,16 +33,16 @@ interface Props {

const NotificationsContext = React.createContext<ContextValue>({
notifications: [],
addError: () => undefined,
addNotification: () => undefined
showError: () => undefined,
showNotification: () => undefined
})

export function NotificationsProvider(props: Props) {
// Not in the state, since state updates would be performed asyncronously
const nextIDRef = useRef(1)
const [notifications, setNotifications] = useState<Notification[]>([])

const addNotification = (type: NotificationType, message: string, options: NotificationOptions = {}) => {
const showNotification = (type: NotificationType, message: string, options: NotificationOptions = {}) => {
const id = nextIDRef.current++

setNotifications(prevNotifications => prevNotifications.concat({ ...options, id, message, type }))
Expand All @@ -51,8 +51,8 @@ export function NotificationsProvider(props: Props) {
setTimeout(() => removeNotificationByID(id), 10000)
}

const addError = (error: any) => {
addNotification("error", String(error.message || error))
const showError = (error: any) => {
showNotification("error", String(error.message || error))

// tslint:disable-next-line:no-console
console.error(error)
Expand All @@ -62,11 +62,11 @@ export function NotificationsProvider(props: Props) {
setNotifications(prevNotifications => prevNotifications.filter(notification => notification.id !== notificationID))
}

trackErrorImplementation = addError
trackErrorImplementation = showError

const contextValue: ContextValue = {
addError,
addNotification,
showError,
showNotification,
notifications
}
return <NotificationsContext.Provider value={contextValue}>{props.children}</NotificationsContext.Provider>
Expand Down
15 changes: 15 additions & 0 deletions src/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ export function useAccountEffectSubscriptions(accounts: Account[], handler: Effe
)
}

export function useOnlineStatus() {
const [isOnline, setOnlineStatus] = useState(window.navigator.onLine)
const setOffline = () => setOnlineStatus(false)
const setOnline = () => setOnlineStatus(true)

useEffect(() => {
window.addEventListener("offline", setOffline)
window.addEventListener("online", setOnline)
}, [])

return {
isOnline
}
}

export function useOrderbook(selling: Asset, buying: Asset, testnet: boolean): ObservedTradingPair {
const horizon = useHorizon(testnet)

Expand Down
16 changes: 16 additions & 0 deletions src/lib/stream.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { trackError } from "../context/notifications"

export function createStreamDebouncer<MessageType>() {
let lastMessageJson = ""
let lastMessageTime = 0
Expand Down Expand Up @@ -31,3 +33,17 @@ export function createStreamDebouncer<MessageType>() {
debounceMessage
}
}

export function trackStreamError(error: Error) {
if (window.navigator.onLine === false) {
// ignore the error if we are offline; the online/offline status is handled separately
return
}

// Wait a little bit, then check again (in case the offline status isn't updated in time)
setTimeout(() => {
if (window.navigator.onLine !== false) {
trackError(error)
}
}, 200)
}
6 changes: 3 additions & 3 deletions src/subscriptions/account-data.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Horizon, Server } from "stellar-sdk"
import { trackError } from "../context/notifications"
import { loadAccount, waitForAccountData } from "../lib/account"
import { createStreamDebouncer } from "../lib/stream"
import { createStreamDebouncer, trackStreamError } from "../lib/stream"
import { createSubscriptionTarget, SubscriptionTarget } from "../lib/subscription"
import { trackError } from "../context/notifications"

export interface ObservedAccountData {
activated: boolean
Expand Down Expand Up @@ -49,7 +49,7 @@ export function createAccountDataSubscription(
},
onerror(error: any) {
debounceError(error, () => {
trackError(new Error("Account data update stream errored."))
trackStreamError(new Error("Account data update stream errored."))
})
}
} as any)
Expand Down
6 changes: 3 additions & 3 deletions src/subscriptions/account-effects.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Server } from "stellar-sdk"
import { trackError } from "../context/notifications"
import { waitForAccountData } from "../lib/account"
import { createStreamDebouncer } from "../lib/stream"
import { createStreamDebouncer, trackStreamError } from "../lib/stream"
import { createSubscriptionTarget, SubscriptionTarget } from "../lib/subscription"
import { trackError } from "../context/notifications"

export function createAccountEffectsSubscription(
horizon: Server,
Expand All @@ -22,7 +22,7 @@ export function createAccountEffectsSubscription(
},
onerror(error: Error) {
debounceError(error, () => {
trackError(new Error("Account effects stream errored."))
trackStreamError(new Error("Account effects stream errored."))
})
}
})
Expand Down
5 changes: 3 additions & 2 deletions src/subscriptions/account-offers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Asset, Server } from "stellar-sdk"
import { trackError } from "../context/notifications"
import { waitForAccountData } from "../lib/account"
import { createSubscriptionTarget, SubscriptionTarget } from "../lib/subscription"
import { trackError } from "../context/notifications"
import { trackStreamError } from "../lib/stream"

export interface ObservedAccountOffers {
loading: boolean
Expand Down Expand Up @@ -47,7 +48,7 @@ export function createAccountOffersSubscription(
const pollingIntervalMs = 5000
setInterval(() => {
if (window.navigator.onLine !== false) {
fetchAccountOffers().catch(trackError)
fetchAccountOffers().catch(trackStreamError)
}
}, pollingIntervalMs)
}
Expand Down
6 changes: 3 additions & 3 deletions src/subscriptions/orderbook.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Asset, Server } from "stellar-sdk"
import { createStreamDebouncer } from "../lib/stream"
import { trackError } from "../context/notifications"
import { createStreamDebouncer, trackStreamError } from "../lib/stream"
import { createSubscriptionTarget, SubscriptionTarget } from "../lib/subscription"
import { FixedOrderbookRecord } from "../lib/orderbook"
import { trackError } from "../context/notifications"

export interface ObservedTradingPair extends FixedOrderbookRecord {
loading: boolean
Expand Down Expand Up @@ -45,7 +45,7 @@ export function createOrderbookSubscription(
// FIXME: We don't want to see errors for every single stream,
// unless it's really a stream-instance-specific error
debounceError(error, () => {
trackError(new Error("Orderbook update stream errored."))
trackStreamError(new Error("Orderbook update stream errored."))
})
}
} as any)
Expand Down
6 changes: 3 additions & 3 deletions src/subscriptions/recent-txs.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Server, Transaction } from "stellar-sdk"
import { trackError } from "../context/notifications"
import { waitForAccountData } from "../lib/account"
import { createStreamDebouncer } from "../lib/stream"
import { createStreamDebouncer, trackStreamError } from "../lib/stream"
import { createSubscriptionTarget, SubscriptionTarget } from "../lib/subscription"
import { trackError } from "../context/notifications"

export interface ObservedRecentTxs {
activated: boolean
Expand Down Expand Up @@ -60,7 +60,7 @@ export function createRecentTxsSubscription(
},
onerror(error: Error) {
debounceError(error, () => {
trackError(new Error("Recent transactions update stream errored."))
trackStreamError(new Error("Recent transactions update stream errored."))
})
}
})
Expand Down
10 changes: 5 additions & 5 deletions stories/Notifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@ storiesOf("Notifications", module).add("All", () => (
<NotificationsProvider>
<NotificationContainer />
<NotificationsContext.Consumer>
{({ addError, addNotification }) => (
{({ showError, showNotification }) => (
<Buttons>
<Button variant="contained" onClick={() => addError(new Error("An error happened."))}>
<Button variant="contained" onClick={() => showError(new Error("An error happened."))}>
Show error notification
</Button>
<Button variant="contained" onClick={() => addNotification("info", "This is an informational message.")}>
<Button variant="contained" onClick={() => showNotification("info", "This is an informational message.")}>
Show info notification
</Button>
<Button variant="contained" onClick={() => addNotification("success", "Action successful!")}>
<Button variant="contained" onClick={() => showNotification("success", "Action successful!")}>
Show success notification
</Button>
<Button
variant="contained"
onClick={() => addNotification("info", "Click me.", { onClick: () => window.alert("Clicked!") })}
onClick={() => showNotification("info", "Click me.", { onClick: () => window.alert("Clicked!") })}
>
Show clickable notification
</Button>
Expand Down

0 comments on commit ee3362b

Please sign in to comment.