Skip to content

Display more helpful websocket errors#1626

Merged
siggy merged 1 commit intomasterfrom
siggy/ws-error-strings
Sep 12, 2018
Merged

Display more helpful websocket errors#1626
siggy merged 1 commit intomasterfrom
siggy/ws-error-strings

Conversation

@siggy
Copy link
Member

@siggy siggy commented Sep 11, 2018

The web client displays Websocket [code] on websocket close errors.

Modify the web client to render a more helpful error message to the
user. If a reason is present, render that, otherwise translate the
websocket error code into a message.

Fixes #1599

Signed-off-by: Andrew Seigner [email protected]

screen shot 2018-09-11 at 2 08 11 pm

@siggy siggy added the area/web label Sep 11, 2018
@siggy siggy self-assigned this Sep 11, 2018
Copy link

@rmars rmars left a comment

Choose a reason for hiding this comment

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

Nice!

this.setState({
error: {
error: `Websocket [${e.code}] ${e.reason}`
error: `Websocket close error: [${e.code}] ${e.reason ? e.reason : wsCloseCodes[e.code]}`
Copy link

Choose a reason for hiding this comment

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

Since the websocket codes are less known that http status codes, it might be worth displaying the text all the time here, like

error: `Websocket close error [${e.code}: ${wsCloseCodes[e.code]}] ${e.reason ? ":" : ""} ${e.reason}`

Copy link
Member Author

Choose a reason for hiding this comment

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

screen shot 2018-09-12 at 10 09 31 am


const maxNumFilterOptions = 12;

// from https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent
Copy link

Choose a reason for hiding this comment

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

There's similar websocket error handling in TopModule.jsx. It might be good to put this into TapUtils.jsx and then we can do the same error handling there.

Copy link
Member Author

Choose a reason for hiding this comment

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

screen shot 2018-09-12 at 10 09 57 am

Copy link

@rmars rmars left a comment

Choose a reason for hiding this comment

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

🌟 nice!

The web client displays `Websocket [code]` on websocket close errors.

Modify the web client to render a more helpful error message to the
user. If a reason is present, render that, otherwise translate the
websocket error code into a message.

Fixes #1599

Signed-off-by: Andrew Seigner <[email protected]>
@siggy siggy force-pushed the siggy/ws-error-strings branch from 2f38646 to deb4702 Compare September 12, 2018 18:19
@siggy siggy merged commit 6c45c07 into master Sep 12, 2018
@siggy siggy deleted the siggy/ws-error-strings branch September 12, 2018 18:29
zachalbert added a commit to zachalbert/linkerd2 that referenced this pull request Sep 13, 2018
* master:
  Update CHANGES.md for v18.9.1 release (linkerd#1631)
  Cleanly shutdown tap stream to data plane proxies (linkerd#1624)
  Change breadcrumb header to default font in styles.css (linkerd#1633)
  Improve top table to better cope with high RPS traffic (linkerd#1634)
  Add small success rate chart to table, misc web tweaks (linkerd#1628)
  Consolidate the source and destination columns in the Tap and Top tables (linkerd#1620)
  remove extraneous calc function in sidebar.css (linkerd#1632)
  Display more helpful websocket errors (linkerd#1626)
  Add breadcrumb navigation at the top of linkerd dashboard (linkerd#1613)
  Introduce inject check for known sidecars (linkerd#1619)
  Bump Prometheus to v2.4.0, Grafana to 5.2.4 (linkerd#1625)
  Improve performance of tap table by throttling updates (linkerd#1623)
  Add with-source flag to top (linkerd#1614)

Conflicts:
	web/app/css/styles.css
	web/app/js/components/ResourceDetail.jsx
	web/app/js/index.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Display more helpful websocket errors

2 participants