Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Better server unavailable error #2419

Merged
merged 2 commits into from Mar 20, 2017
Merged

Conversation

ianb
Copy link
Contributor

@ianb ianb commented Mar 17, 2017

The error handling wasn't really working here

req.onerror is needed to catch connection denied (as opposed to a non-200 request error)
@jaredhirsch jaredhirsch self-requested a review March 20, 2017 20:44
@jaredhirsch
Copy link
Member

@ianb Hmm, the notification looks a bit odd:

screen shot 2017-03-20 at 1 46 58 pm

@ianb
Copy link
Contributor Author

ianb commented Mar 20, 2017

I suppose we could strip /^error:\s+/i from the message

@jaredhirsch
Copy link
Member

I'm seeing that busted notification in a few other spots, so I"ll just file a bug about it and finish this review

Copy link
Member

@jaredhirsch jaredhirsch left a comment

Choose a reason for hiding this comment

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

I'm seeing a nicer 'Cannot connect to the page shot server' error, followed about half a second later by a Link Copied notification (with undefined as the string in the clipboard).

This code looks good, and the Link Copied error doesn't seem related. I'll file a followup bug to not show the Link Copied notification if there's no link.

@jaredhirsch jaredhirsch merged commit b030708 into master Mar 20, 2017
@jaredhirsch jaredhirsch deleted the better-server-unavailable-error branch March 20, 2017 23:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants