Repository: illinois/queue
Pull request #263 (2019-04-08 04:05:05)

Rewrite queue page with hooks
~My hope is that this rewrite will help eliminate the 404s we've been seeing at random. My best guess (or, hunch, really) as to why it's happening is a race condition of some kind, which hooks are better at avoiding. We also explicitly maintain our own loading state in the component. Again, this is just a hunch, and we'll have to see if the anecdotal reports of 404s stop after this.~

Turns out my initial hunch was totally wrong. Eddie figured out how to reproduce the 404 bug:

* View a queue on Safari on an iPhone
* Lock the device
* Unlock the device

It appears as though Safari kills socket connections when Safari isn't in the foreground. When that would happen, we'd get an error event, which (due to my bad error handling) would cause another error. That error would get picked up by React and rendered as our custom error page by Next.js. Unfortunately, our custom error page didn't work well for this use case, because it would default to showing a misleading 404 if a status code wasn't present on the error (which it wasn't, in this case).

So, this PR does a few things to address these issues:

* Properly handles websocket errors
* Reports websocket state to users (reconnecting, connected, failed)
* Renders appropriate error message for non-HTTP-status errors

Given the context of the original problem, would be awesome if people have iOS devices to test with.

Also fixes or enhances a few other things:

* Fixes a regression introduced in #233 where the `id`s of `activeStaff` models weren't being included in the "get queue" response; adds test to prevent future regressions
* Changes how course name is displayed on queue pages, which now serves as a link to the appropriate course
* Limits socket reconnection attempts to 5
* Explicitly handle socket authentication failures separately from normal socket errors.


now[bot]: 
This pull request is [automatically deployed](https://zeit.co/docs/features/now-for-github?utm_source=automated&utm_medium=github&utm_campaign=now_bot) with [Now](https://zeit.co/now?utm_source=automated&utm_medium=github&utm_campaign=now_bot).
To access deployments, click Details below or on the icon next to each push.
	

nwalters512: Also tested this on Now with Eddie's iPad - it correctly reports that the socket is disconnected, then connecting, then connected again after the device is awoken from sleep.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e99f806..8a5edd6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ with the current semantic version and the next changes should go under a **[Next
 * Add logging for new socket initialization error. ([@nwalters512](https://github.com/nwalters512) in [#258](https://github.com/illinois/queue/pull/258))
 * Remove unused style tag. ([@nwalters512](https://github.com/nwalters512) in [#259](https://github.com/illinois/queue/pull/259))
 * Fix active staff socket errors. ([@nwalters512](https://github.com/nwalters512) in [#262](https://github.com/illinois/queue/pull/262))
+* Rewrite queue page with hooks; fix miscellaneous bugs. ([@nwalters512](https://github.com/nwalters512) in [#263](https://github.com/illinois/queue/pull/263))
 
 ## v1.0.4
 
diff --git a/src/actions/queue.js b/src/actions/queue.js
index 7f4f379..e01447f 100644
--- a/src/actions/queue.js
+++ b/src/actions/queue.js
@@ -63,7 +63,6 @@ export function createQueue(courseId, queue) {
 /**
  * Fetch a queue
  */
-
 export const fetchQueueRequest = makeActionCreator(
   types.FETCH_QUEUE.REQUEST,
   'queueId'
@@ -88,7 +87,7 @@ export function fetchQueue(queueId) {
       .then(res => dispatch(fetchQueueSuccess(queueId, res.data)))
       .catch(err => {
         console.error(err)
-        dispatch(fetchQueueFailure(queueId, err))
+        return dispatch(fetchQueueFailure(queueId, err))
       })
   }
 }
diff --git a/src/actions/socket.js b/src/actions/socket.js
index 30014fa..8af006c 100644
--- a/src/actions/socket.js
+++ b/src/actions/socket.js
@@ -1,8 +1,6 @@
 import { makeActionCreator } from './util'
 import * as types from '../constants/ActionTypes'
 
-export const setSocketError = makeActionCreator(types.SET_SOCKET_ERROR, 'error')
-
 export const setSocketStatus = makeActionCreator(
   types.SET_SOCKET_STATUS,
   'status'
diff --git a/src/api/queues.js b/src/api/queues.js
index 05cd9f6..4445f92 100644
--- a/src/api/queues.js
+++ b/src/api/queues.js
@@ -104,7 +104,7 @@ router.get(
           },
           required: false,
           include: [User],
-          attributes: ['startTime', 'endTime'],
+          attributes: ['id', 'startTime', 'endTime'],
         },
         {
           model: Question,
diff --git a/src/components/Error.js b/src/components/Error.js
index b448383..6ba1c87 100644
--- a/src/components/Error.js
+++ b/src/components/Error.js
@@ -34,15 +34,20 @@ const styles = {
 
 const Error = props => {
   const { statusCode } = props
-  const title =
-    statusCode === 404
-      ? 'This page could not be found'
-      : HTTPStatus[statusCode] || 'An unexpected error has occurred'
+  const title = statusCode !== null ? statusCode : 'RIP'
+  let message
+  if (statusCode) {
+    message = HTTPStatus[statusCode] || 'RIP'
+  } else if (props.message) {
+    message = props.message || 'An unexpected error occurred'
+  } else {
+    message = 'An unexpected error occurred'
+  }
 
   return (
     <div style={styles.error}>
-      <h1 className="display-2">{statusCode || 'Error!'}</h1>
-      <h6>{title}.</h6>
+      <h1 className="display-2">{title}</h1>
+      <h6>{message}</h6>
       <Link passHref route="index">
         <Button outline color="secondary" tag="a" className="mt-4">
           Go to homepage
@@ -54,10 +59,12 @@ const Error = props => {
 
 Error.defaultProps = {
   statusCode: 404,
+  message: null,
 }
 
 Error.propTypes = {
   statusCode: PropTypes.number,
+  message: PropTypes.string,
 }
 
 export default Error
diff --git a/src/components/QueueMessageViewer.js b/src/components/QueueMessageViewer.js
index a6b1e28..cc94532 100644
--- a/src/components/QueueMessageViewer.js
+++ b/src/components/QueueMessageViewer.js
@@ -110,10 +110,14 @@ const QueueMessageViewer = props => {
 
 QueueMessageViewer.propTypes = {
   queueId: PropTypes.number.isRequired,
-  message: PropTypes.string.isRequired,
+  message: PropTypes.string,
   collapsible: PropTypes.bool.isRequired,
   editable: PropTypes.bool.isRequired,
   onEdit: PropTypes.func.isRequired,
 }
 
+QueueMessageViewer.defaultProps = {
+  message: '',
+}
+
 export default QueueMessageViewer
diff --git a/src/components/SocketErrorModal.jsx b/src/components/SocketErrorModal.jsx
index 4cac10c..4b46b61 100644
--- a/src/components/SocketErrorModal.jsx
+++ b/src/components/SocketErrorModal.jsx
@@ -2,8 +2,8 @@ import React from 'react'
 import PropTypes from 'prop-types'
 import { Modal, ModalHeader, ModalBody } from 'reactstrap'
 
-const SocketErrorModal = ({ isOpen, toggle, error }) => (
-  <Modal isOpen={isOpen} toggle={toggle}>
+const SocketErrorModal = ({ isOpen }) => (
+  <Modal isOpen={isOpen}>
     <ModalHeader>Socket error</ModalHeader>
     <ModalBody>
       <p>
@@ -14,28 +14,12 @@ const SocketErrorModal = ({ isOpen, toggle, error }) => (
         developers can get in touch with you! In the meantime, please try
         accessing the Queue from another browser or device.
       </p>
-      {error && (
-        <>
-          <p>
-            <b>Please include the below message in your email:</b>
-          </p>
-          <pre>
-            <code>{error}</code>
-          </pre>
-        </>
-      )}
     </ModalBody>
   </Modal>
 )
 
 SocketErrorModal.propTypes = {
   isOpen: PropTypes.bool.isRequired,
-  toggle: PropTypes.func.isRequired,
-  error: PropTypes.string,
-}
-
-SocketErrorModal.defaultProps = {
-  error: null,
 }
 
 export default SocketErrorModal
diff --git a/src/components/SocketStatusAlert.jsx b/src/components/SocketStatusAlert.jsx
new file mode 100644
index 0000000..29b2343
--- /dev/null
+++ b/src/components/SocketStatusAlert.jsx
@@ -0,0 +1,48 @@
+import React from 'react'
+import PropTypes from 'prop-types'
+import { Alert, Collapse } from 'reactstrap'
+import {
+  SOCKET_CONNECTED,
+  SOCKET_CONNECTING,
+  SOCKET_ERROR,
+  SOCKET_STATUS_TYPES,
+} from '../constants/socketStatus'
+
+const SocketStatusAlert = ({ isOpen, status }) => {
+  let message
+  let color
+  switch (status) {
+    case SOCKET_CONNECTED:
+      message = 'Connected'
+      color = 'success'
+      break
+    case SOCKET_CONNECTING:
+      message = 'Trying to connect...'
+      color = 'warning'
+      break
+    case SOCKET_ERROR:
+      message = 'Lost connection. Check your network and refresh the page.'
+      color = 'danger'
+      break
+    default:
+      return null
+  }
+  return (
+    <Collapse isOpen={isOpen}>
+      <Alert fade={false} color={color}>
+        {message}
+      </Alert>
+    </Collapse>
+  )
+}
+
+SocketStatusAlert.propTypes = {
+  isOpen: PropTypes.bool.isRequired,
+  status: PropTypes.oneOf(SOCKET_STATUS_TYPES),
+}
+
+SocketStatusAlert.defaultProps = {
+  status: null,
+}
+
+export default SocketStatusAlert
diff --git a/src/components/StaffSidebar.js b/src/components/StaffSidebar.js
index ea1a8ee..6e7b5cc 100644
--- a/src/components/StaffSidebar.js
+++ b/src/components/StaffSidebar.js
@@ -58,6 +58,7 @@ class StaffSidebar extends React.Component {
     if (this.props.queue) {
       const { removeStaff, queue } = this.props
       const activeStaffIds = queue.activeStaff
+
       if (activeStaffIds && activeStaffIds.length > 0) {
         staffList = activeStaffIds.map(id => {
           const activeStaffId = id
diff --git a/src/constants/ActionTypes.js b/src/constants/ActionTypes.js
index 3fcd882..17a3935 100644
--- a/src/constants/ActionTypes.js
+++ b/src/constants/ActionTypes.js
@@ -45,5 +45,4 @@ export const REPLACE_ACTIVE_STAFF = 'REPLACE_ACTIVE_STAFF'
 
 /* Actions for socket status */
 export const SET_SOCKET_STATUS = 'SET_SOCKET_STATUS'
-export const SET_SOCKET_ERROR = 'SET_SOCKET_ERROR'
 export const RESET_SOCKET_STATE = 'RESET_SOCKET_STATE'
diff --git a/src/constants/socketStatus.js b/src/constants/socketStatus.js
new file mode 100644
index 0000000..6d692f1
--- /dev/null
+++ b/src/constants/socketStatus.js
@@ -0,0 +1,10 @@
+export const SOCKET_CONNECTING = 'connecting'
+export const SOCKET_CONNECTED = 'connected'
+export const SOCKET_ERROR = 'error'
+export const SOCKET_AUTHENTICATION_ERROR = 'authentication_error'
+export const SOCKET_STATUS_TYPES = [
+  SOCKET_CONNECTING,
+  SOCKET_CONNECTED,
+  SOCKET_ERROR,
+  SOCKET_AUTHENTICATION_ERROR,
+]
diff --git a/src/pages/_error.js b/src/pages/_error.js
index 58a4afe..9dc792f 100644
--- a/src/pages/_error.js
+++ b/src/pages/_error.js
@@ -1,3 +1,4 @@
+/* eslint-disable prefer-destructuring */
 import React from 'react'
 import PropTypes from 'prop-types'
 
@@ -7,26 +8,34 @@ import PageWithUser from '../components/PageWithUser'
 export class ErrorPage extends React.Component {
   static getInitialProps({ res, err }) {
     let statusCode = null
+    let message = null
     if (res) {
-      ;({ statusCode } = res)
+      statusCode = res.statusCode
     } else if (err) {
-      ;({ statusCode } = err)
+      if (err.statusCode) {
+        statusCode = err.statusCode
+      } else {
+        message = err.message
+      }
     }
 
-    return { statusCode }
+    return { statusCode, message }
   }
 
   render() {
-    return <Error statusCode={this.props.statusCode} />
+    const { statusCode, message } = this.props
+    return <Error statusCode={statusCode} message={message} />
   }
 }
 
 ErrorPage.defaultProps = {
-  statusCode: 404,
+  statusCode: null,
+  message: null,
 }
 
 ErrorPage.propTypes = {
   statusCode: PropTypes.number,
+  message: PropTypes.string,
 }
 
 export default PageWithUser(ErrorPage)
diff --git a/src/pages/queue.js b/src/pages/queue.js
index eadccdd..84e02b8 100644
--- a/src/pages/queue.js
+++ b/src/pages/queue.js
@@ -1,6 +1,7 @@
-import React from 'react'
+import React, { useState, useEffect } from 'react'
 import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
+import { usePrevious } from 'react-hanger'
 import {
   Container,
   Row,
@@ -13,7 +14,8 @@ import {
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
 import { faMapMarker, faEyeSlash } from '@fortawesome/free-solid-svg-icons'
 
-import { fetchQueue, fetchQueueRequest } from '../actions/queue'
+import { Link } from '../routes'
+import { fetchQueue } from '../actions/queue'
 import { fetchCourse } from '../actions/course'
 import { connectToQueue, disconnectFromQueue } from '../socket/client'
 
@@ -32,144 +34,177 @@ import { isUserCourseStaffForQueue, isUserAdmin } from '../selectors'
 import ConfidentialQueuePanelContainer from '../containers/ConfidentialQueuePanelContainer'
 import SocketErrorModal from '../components/SocketErrorModal'
 import { resetSocketState } from '../actions/socket'
+import { FETCH_QUEUE } from '../constants/ActionTypes'
+import {
+  SOCKET_CONNECTING,
+  SOCKET_CONNECTED,
+  SOCKET_ERROR,
+  SOCKET_AUTHENTICATION_ERROR,
+} from '../constants/socketStatus'
+import SocketStatusAlert from '../components/SocketStatusAlert'
 
-class Queue extends React.Component {
-  static getInitialProps({ isServer, store, query }) {
-    const queueId = Number.parseInt(query.id, 10)
-    if (isServer) {
-      store.dispatch(fetchQueueRequest(queueId))
-    }
-    return {
-      queueId,
-      isFetchingQueue: isServer,
-    }
-  }
+const buildQueueName = (queue, course) => {
+  return (
+    <>
+      {course && (
+        <>
+          <Link route="course" params={{ id: course.id }}>
+            {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
+            <a>{course.name}</a>
+          </Link>
+          <span className="mx-1">/</span>
+        </>
+      )}
+      {queue.name}
+    </>
+  )
+}
 
-  static pageTransitionDelayEnter = true
+const Queue = props => {
+  const [queueLoading, setQueueLoading] = useState(true)
+  const [showSocketStatus, setShowSocketStatus] = useState(false)
+  const previousSocketStatus = usePrevious(props.socketStatus)
 
-  componentDidMount() {
-    this.props.fetchQueue(this.props.queueId).then(action => {
-      if (this.props.pageTransitionReadyToEnter) {
-        this.props.pageTransitionReadyToEnter()
+  useEffect(() => {
+    setQueueLoading(true)
+    props.fetchQueue(props.queueId).then(action => {
+      setQueueLoading(false)
+      if (props.pageTransitionReadyToEnter) {
+        props.pageTransitionReadyToEnter()
+      }
+      if (action.type === FETCH_QUEUE.SUCCESS) {
+        props.fetchCourse(action.queue.courseId)
       }
-      // We won't block the page from showing while we load the course - we'll
-      // simply show the course name as soon as it's available
-      return this.props.fetchCourse(action.queue.courseId)
     })
-  }
+  }, [props.queueId])
 
-  componentDidUpdate(prevProps) {
+  useEffect(() => {
+    connectToQueue(props.dispatch, props.queueId)
+    return () => {
+      disconnectFromQueue(props.queueId)
+      resetSocketState()
+    }
+  }, [props.queueId])
+
+  useEffect(() => {
+    const NOOP = () => {}
+    if (previousSocketStatus === props.socketStatus) return NOOP
     if (
-      prevProps.isFetchingQueue &&
-      !this.props.isFetchingQueue &&
-      this.props.hasQueue
+      previousSocketStatus === SOCKET_CONNECTED &&
+      props.socketStatus !== SOCKET_CONNECTED
     ) {
-      // We have finished fetching the queue and the queue exists (was not a 404)
-      // It's now safe to connect to the websocket
-      connectToQueue(this.props.dispatch, this.props.queueId)
+      setShowSocketStatus(true)
+      return NOOP
     }
-  }
+    if (
+      props.socketStatus === SOCKET_ERROR ||
+      props.socketStatus === SOCKET_AUTHENTICATION_ERROR
+    ) {
+      setShowSocketStatus(true)
+      return NOOP
+    }
+    if (props.socketStatus === SOCKET_CONNECTED) {
+      const timeoutId = setTimeout(() => {
+        setShowSocketStatus(false)
+      }, 1000)
+      return () => clearTimeout(timeoutId)
+    }
+    return NOOP
+  }, [props.socketStatus])
 
-  componentWillUnmount() {
-    disconnectFromQueue(this.props.queueId)
-    resetSocketState()
+  if (queueLoading) {
+    return null
   }
 
-  render() {
-    const { isFetchingQueue, hasQueue } = this.props
+  if (!queueLoading && !props.queue) {
+    return <Error statusCode={404} />
+  }
 
-    if (isFetchingQueue) {
-      return null
-    }
-    if (!isFetchingQueue && !hasQueue) {
-      return <Error statusCode={404} />
-    }
-    const locationText = this.props.queue.location || 'No location specified'
-    const confidentialMessage =
-      this.props.isUserCourseStaff || this.props.isUserAdmin
-        ? 'Students'
-        : 'You'
-
-    let queueName = ''
-    const { name } = this.props.queue
-    if (this.props.course) {
-      queueName += `${this.props.course.name} — `
-    }
-    queueName += name
-    return (
-      <Container fluid>
-        <h3>
-          {this.props.queue.isConfidential && (
-            <span>
-              <FontAwesomeIcon
-                icon={faEyeSlash}
-                fixedWidth
-                className="mr-2"
-                id="confidentialIcon"
-              />
-              <UncontrolledTooltip placement="bottom" target="confidentialIcon">
-                This is a confidential queue! {confidentialMessage} won&apos;t
-                be able to see questions from other students.
-              </UncontrolledTooltip>
-            </span>
-          )}
-          {queueName}
-        </h3>
-        <h5 className="mb-3 text-muted">
-          <FontAwesomeIcon icon={faMapMarker} fixedWidth className="mr-2" />
-          {locationText}
-        </h5>
-        <Row>
-          <Col
-            xs={{ size: 12 }}
-            md={{ size: 4 }}
-            lg={{ size: 3 }}
-            className="mb-3 mb-md-0"
-          >
-            <QuestionNotificationsToggle />
-            <ShowForCourseStaff queueId={this.props.queueId}>
-              <QueueStatusToggleContainer queue={this.props.queue} />
-              {!this.props.queue.open && (
-                <DeleteAllQuestionsButtonContainer queue={this.props.queue} />
-              )}
-              <QueueMessageEnabledToggleContainer queue={this.props.queue} />
-            </ShowForCourseStaff>
-            <StaffSidebar queueId={this.props.queueId} />
-          </Col>
-          <Col xs={{ size: 12 }} md={{ size: 8 }} lg={{ size: 9 }}>
-            <Collapse isOpen={this.props.queue.messageEnabled}>
-              <QueueMessageContainer queueId={this.props.queueId} />
-            </Collapse>
-            {this.props.queue.open && (
-              <QuestionPanelContainer queueId={this.props.queueId} />
+  const queueName = buildQueueName(props.queue, props.course)
+  const locationText = props.queue.location || 'No location specified'
+  const confidentialMessage =
+    props.isUserCourseStaff || props.isUserAdmin ? 'Students' : 'You'
+  return (
+    <Container fluid>
+      <SocketStatusAlert
+        status={props.socketStatus}
+        isOpen={showSocketStatus}
+      />
+      <h3>
+        {props.queue.isConfidential && (
+          <span>
+            <FontAwesomeIcon
+              icon={faEyeSlash}
+              fixedWidth
+              className="mr-2"
+              id="confidentialIcon"
+            />
+            <UncontrolledTooltip placement="bottom" target="confidentialIcon">
+              This is a confidential queue! {confidentialMessage} won&apos;t be
+              able to see questions from other students.
+            </UncontrolledTooltip>
+          </span>
+        )}
+        {queueName}
+      </h3>
+      <h5 className="mb-3 text-muted">
+        <FontAwesomeIcon icon={faMapMarker} fixedWidth className="mr-2" />
+        {locationText}
+      </h5>
+      <Row>
+        <Col
+          xs={{ size: 12 }}
+          md={{ size: 4 }}
+          lg={{ size: 3 }}
+          className="mb-3 mb-md-0"
+        >
+          <QuestionNotificationsToggle />
+          <ShowForCourseStaff queueId={props.queueId}>
+            <QueueStatusToggleContainer queue={props.queue} />
+            {!props.queue.open && (
+              <DeleteAllQuestionsButtonContainer queue={props.queue} />
             )}
-            {!this.props.queue.open && (
-              <Card className="bg-light mb-3">
-                <CardBody className="text-center">
-                  This queue is closed. Check back later!
-                </CardBody>
-              </Card>
+            <QueueMessageEnabledToggleContainer queue={props.queue} />
+          </ShowForCourseStaff>
+          <StaffSidebar queueId={props.queueId} />
+        </Col>
+        <Col xs={{ size: 12 }} md={{ size: 8 }} lg={{ size: 9 }}>
+          <Collapse isOpen={props.queue.messageEnabled}>
+            <QueueMessageContainer queueId={props.queueId} />
+          </Collapse>
+          {props.queue.open && (
+            <QuestionPanelContainer queueId={props.queueId} />
+          )}
+          {!props.queue.open && (
+            <Card className="bg-light mb-3">
+              <CardBody className="text-center">
+                This queue is closed. Check back later!
+              </CardBody>
+            </Card>
+          )}
+          {props.queue.isConfidential &&
+            !props.isUserCourseStaff &&
+            !props.isUserAdmin && (
+              <ConfidentialQueuePanelContainer queueId={props.queueId} />
             )}
-            {this.props.queue.isConfidential &&
-              !this.props.isUserCourseStaff &&
-              !this.props.isUserAdmin && (
-                <ConfidentialQueuePanelContainer queueId={this.props.queueId} />
-              )}
-            <QuestionListContainer queueId={this.props.queueId} />
-          </Col>
-        </Row>
-        <SocketErrorModal
-          isOpen={!!this.props.socketError}
-          error={this.props.socketError}
-        />
-      </Container>
-    )
-  }
+          <QuestionListContainer queueId={props.queueId} />
+        </Col>
+      </Row>
+      <SocketErrorModal
+        isOpen={props.socketStatus === SOCKET_AUTHENTICATION_ERROR}
+      />
+    </Container>
+  )
 }
 
+Queue.getInitialProps = async ({ query }) => {
+  const queueId = Number.parseInt(query.id, 10)
+  return { queueId }
+}
+
+Queue.pageTransitionDelayEnter = true
+
 Queue.propTypes = {
-  isFetchingQueue: PropTypes.bool.isRequired,
-  hasQueue: PropTypes.bool.isRequired,
   fetchQueue: PropTypes.func.isRequired,
   fetchCourse: PropTypes.func.isRequired,
   queueId: PropTypes.number.isRequired,
@@ -190,28 +225,25 @@ Queue.propTypes = {
     name: PropTypes.string,
   }),
   pageTransitionReadyToEnter: PropTypes.func,
-  socketError: PropTypes.string,
+  socketStatus: PropTypes.string,
 }
 
 Queue.defaultProps = {
   queue: null,
   course: null,
   pageTransitionReadyToEnter: null,
-  socketError: null,
+  socketStatus: SOCKET_CONNECTING,
 }
 
 const mapStateToProps = (state, ownProps) => {
   const queue = state.queues.queues[ownProps.queueId]
   const course = queue && state.courses.courses[queue.courseId]
   return {
-    isFetchingQueue: state.queues.isFetching,
-    hasQueue: !!queue,
-    hasCourse: !!course,
     queue,
     course,
     isUserCourseStaff: isUserCourseStaffForQueue(state, ownProps),
     isUserAdmin: isUserAdmin(state, ownProps),
-    socketError: state.socket.error,
+    socketStatus: state.socket.status,
   }
 }
 
diff --git a/src/reducers/queues.js b/src/reducers/queues.js
index 59bd9e7..72424b1 100644
--- a/src/reducers/queues.js
+++ b/src/reducers/queues.js
@@ -100,8 +100,6 @@ function addActiveStaffToQueue(state, queueId, activeStaffId) {
   return newState
 }
 
-// We need to extract the queues from
-
 const queues = (state = defaultState, action) => {
   switch (action.type) {
     case FETCH_COURSE.REQUEST: {
@@ -117,8 +115,12 @@ const queues = (state = defaultState, action) => {
         queues: {
           ...state.queues,
           ...action.course.queues.reduce((obj, item) => {
+            const newQueue = normalizeQueue(item)
+            // We'll merge in any new info about this queue but keep existing
+            // info, since the queue models returned in course requests aren't
+            // complete
             // eslint-disable-next-line no-param-reassign
-            obj[item.id] = normalizeQueue(item)
+            obj[item.id] = { ...state.queues[item.id], ...newQueue }
             return obj
           }, {}),
         },
diff --git a/src/reducers/socket.js b/src/reducers/socket.js
index 55d1dff..d07ccd7 100644
--- a/src/reducers/socket.js
+++ b/src/reducers/socket.js
@@ -1,12 +1,7 @@
-import {
-  SET_SOCKET_STATUS,
-  SET_SOCKET_ERROR,
-  RESET_SOCKET_STATE,
-} from '../constants/ActionTypes'
+import { SET_SOCKET_STATUS, RESET_SOCKET_STATE } from '../constants/ActionTypes'
 
 const defaultState = {
   status: null,
-  error: null,
 }
 
 const socket = (state = defaultState, action) => {
@@ -16,11 +11,6 @@ const socket = (state = defaultState, action) => {
         ...state,
         status: action.status,
       }
-    case SET_SOCKET_ERROR:
-      return {
-        ...state,
-        error: action.error,
-      }
     case RESET_SOCKET_STATE:
       return {
         ...defaultState,
diff --git a/src/socket/client.js b/src/socket/client.js
index 4092673..e8d4bff 100644
--- a/src/socket/client.js
+++ b/src/socket/client.js
@@ -13,10 +13,17 @@ import {
 import { replaceActiveStaff } from '../actions/activeStaff'
 import { normalizeActiveStaff } from '../reducers/normalize'
 import { baseUrl } from '../util'
-import { setSocketError } from '../actions/socket'
+import { setSocketStatus } from '../actions/socket'
+import {
+  SOCKET_ERROR,
+  SOCKET_CONNECTING,
+  SOCKET_CONNECTED,
+  SOCKET_AUTHENTICATION_ERROR,
+} from '../constants/socketStatus'
 
 const socketOpts = {
   path: `${baseUrl}/socket.io`,
+  reconnectionAttempts: 5,
 }
 
 const queueSockets = {}
@@ -52,18 +59,28 @@ export const connectToQueue = (dispatch, queueId) => {
   queueSockets[queueId] = socket
   socket.on('connect', () => {
     socket.emit('join', { queueId }, ({ questions, activeStaff }) => {
+      dispatch(setSocketStatus(SOCKET_CONNECTED))
       dispatch(replaceQuestions(queueId, questions))
       dispatch(replaceActiveStaff(queueId, activeStaff))
     })
   })
-  socket.on('connect_failed', err => {
-    dispatch(setSocketError(err))
-    console.error(err)
-  })
   socket.on('error', err => {
-    dispatch(setSocketError(err))
+    if (err === 'Authentication error') {
+      dispatch(setSocketStatus(SOCKET_AUTHENTICATION_ERROR))
+    } else {
+      dispatch(setSocketStatus(SOCKET_ERROR))
+    }
     console.error(err)
   })
+  socket.on('reconnecting', () => {
+    dispatch(setSocketStatus(SOCKET_CONNECTING))
+  })
+  socket.on('reconnect', () => {
+    dispatch(setSocketStatus(SOCKET_CONNECTED))
+  })
+  socket.on('reconnect_failed', () => {
+    dispatch(setSocketStatus(SOCKET_ERROR))
+  })
   socket.on('question:create', ({ question }) =>
     handleQuestionCreate(dispatch, queueId, question)
   )
diff --git a/src/socket/server.js b/src/socket/server.js
index f8394a8..0849e91 100644
--- a/src/socket/server.js
+++ b/src/socket/server.js
@@ -214,7 +214,7 @@ module.exports = newIo => {
     if (!user) {
       console.error('failed to authenticate socket')
       console.error(`jwt cookie present? ${!!jwtCookie}`)
-      next(new Error('Could not authenticate socket connection'))
+      next(new Error('Authentication error'))
     } else {
       // eslint-disable-next-line no-param-reassign
       socket.request.user = user
@@ -238,6 +238,10 @@ module.exports = newIo => {
             queuePromise,
             userAuthzPromise,
           ])
+          if (!queue) {
+            // User tried to connect to a non-existent queue
+            return
+          }
           const { courseId, isConfidential } = queue
           const isStudent = isUserStudent(userAuthz, courseId)
           let sendCompleteQuestionData = true


Test patch:
diff --git a/src/api/queues.test.js b/src/api/queues.test.js
index ac2b080..020a915 100644
--- a/src/api/queues.test.js
+++ b/src/api/queues.test.js
@@ -53,75 +53,67 @@ describe('Queues API', () => {
   })
 
   describe('GET /api/queues/2', () => {
-    test('succeeds for admin', async () => {
-      const request = await requestAsUser(app, 'admin')
+    const testForUser = async (username, isAdmin) => {
+      const request = await requestAsUser(app, username)
+      const staffRequest = await requestAsUser(app, '241staff')
       const res = await request.get('/api/queues/2')
       expect(res.statusCode).toBe(200)
       expect(res.body.id).toBe(2)
       expect(res.body.name).toBe('CS241 Queue')
       expect(res.body.location).toBe('There')
       expect(res.body.courseId).toBe(2)
-
       expect(res.body).toHaveProperty('questions')
       expect(res.body.questions).toHaveLength(0)
       expect(res.body).toHaveProperty('activeStaff')
       expect(res.body.activeStaff).toHaveLength(0)
-      includesPrivateAttributes(res.body)
+      if (isAdmin) {
+        includesPrivateAttributes(res.body)
+      } else {
+        excludesPrivateAttributes(res.body)
+      }
+
+      // Have 241 staff member join the queue staff
+      const res2 = await staffRequest.post('/api/queues/2/staff/3')
+      const activeStaffId = res2.body.id
 
+      // Add a question
       const question = { name: 'a', location: 'b', topic: 'c' }
-      const res2 = await request.post('/api/queues/2/questions').send(question)
-      expect(res2.body.askedBy.netid).toBe('admin')
-      const questionId = res2.body.id
+      const res3 = await request.post('/api/queues/2/questions').send(question)
+      expect(res3.body.askedBy.netid).toBe(username)
+      const questionId = res3.body.id
 
-      const res3 = await request.post(
+      // Answer the question
+      const res4 = await staffRequest.post(
         `/api/queues/2/questions/${questionId}/answering`
       )
-      expect(res3.body.answeredBy.name).toBe('Admin')
+      expect(res4.body.answeredBy.name).toBe('241 Staff')
+
+      const res5 = await request.get('/api/queues/2')
+      expect(res5.body).toHaveProperty('questions')
+      expect(res5.body.questions).toHaveLength(1)
+      expect(res5.body.questions[0]).toHaveProperty('askedBy')
+      expect(res5.body.questions[0].askedBy.netid).toBe(username)
+      expect(res5.body.questions[0]).toHaveProperty('answeredBy')
+      expect(res5.body.questions[0].answeredBy.name).toBe('241 Staff')
+      expect(res5.body).toHaveProperty('activeStaff')
+      expect(res5.body.activeStaff).toHaveLength(1)
+      expect(res5.body.activeStaff[0]).toHaveProperty('id')
+      expect(res5.body.activeStaff[0].id).toEqual(activeStaffId)
+      expect(res5.body.activeStaff[0].user.netid).toEqual('241staff')
+      expect(res5.body.activeStaff[0].user.name).toEqual('241 Staff')
+      if (isAdmin) {
+        includesPrivateAttributes(res5.body)
+      } else {
+        excludesPrivateAttributes(res5.body)
+      }
+    }
 
-      const res4 = await request.get('/api/queues/2')
-      expect(res4.body).toHaveProperty('questions')
-      expect(res4.body.questions).toHaveLength(1)
-      expect(res4.body.questions[0]).toHaveProperty('askedBy')
-      expect(res4.body.questions[0].askedBy.netid).toBe('admin')
-      expect(res4.body.questions[0]).toHaveProperty('answeredBy')
-      expect(res4.body.questions[0].answeredBy.name).toBe('Admin')
-      includesPrivateAttributes(res4.body)
+    test('succeeds for admin', async () => {
+      await testForUser('admin', true)
     })
 
-    test('succeeds for non-admin', async () => {
-      const request = await requestAsUser(app, 'student')
-      const res = await request.get('/api/queues/2')
-      expect(res.statusCode).toBe(200)
-      expect(res.body.id).toBe(2)
-      expect(res.body.name).toBe('CS241 Queue')
-      expect(res.body.location).toBe('There')
-      expect(res.body.courseId).toBe(2)
-
-      expect(res.body).toHaveProperty('questions')
-      expect(res.body.questions).toHaveLength(0)
-      expect(res.body).toHaveProperty('activeStaff')
-      expect(res.body.activeStaff).toHaveLength(0)
-      excludesPrivateAttributes(res.body)
-
-      const question = { name: 'a', location: 'b', topic: 'c' }
-      const request2 = await requestAsUser(app, 'admin')
-      const res2 = await request2.post('/api/queues/2/questions').send(question)
-      expect(res2.body.askedBy.netid).toBe('admin')
-      const questionId = res2.body.id
-
-      const res3 = await request2.post(
-        `/api/queues/2/questions/${questionId}/answering`
-      )
-      expect(res3.body.answeredBy.name).toBe('Admin')
-
-      const res4 = await request.get('/api/queues/2')
-      expect(res4.body).toHaveProperty('questions')
-      expect(res4.body.questions).toHaveLength(1)
-      expect(res4.body.questions[0]).toHaveProperty('askedBy')
-      expect(res4.body.questions[0].askedBy.netid).toBe('admin')
-      expect(res4.body.questions[0]).toHaveProperty('answeredBy')
-      expect(res4.body.questions[0].answeredBy.name).toBe('Admin')
-      excludesPrivateAttributes(res4.body)
+    test('succeeds for student', async () => {
+      await testForUser('student', false)
     })
   })