Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix json parsing exceptions obscuring server errors #1605

Merged
merged 1 commit into from Oct 24, 2017

Conversation

liliakai
Copy link
Contributor

I got a 413 (Rate limit exceeded) error from the server while fetching prekeys. The client tried to parse the response as json since we expect json from the prekey endpoint, which threw an exception because the response is not json in this case.

This caused us to present the failure as a general network failure instead of a rate limit issue. The overall impact on UX was minimal, since the resend button appeared and worked as expected, but the error message was wrong.

This change prevents us from treating the response as json unless it has the Content-Type header set accordingly. If for some reason, the client and server disagree on whether the response
should be or is json, we'll default to treating it as text.

// FREEBIE

I got a 413 (Rate limit exceeded) error from the server while fetching prekeys.
The client tried to parse the response as json since we expect json from the
prekey endpoint, which threw an exception because the response was not json.
This change prevents us from treating the response as json unless it has the
Content-Type header set accordingly.

If for some reason, the client and server disagree on whether the response
should be or is json, we'll default to treating it as text.

// FREEBIE
@scottnonnenberg scottnonnenberg merged commit d1f7f5e into master Oct 24, 2017
@scottnonnenberg scottnonnenberg deleted the nodeFetchBug branch October 24, 2017 22:54
scottnonnenberg added a commit that referenced this pull request Oct 30, 2017
Emoji picker (#1608)

Prevent drawAttention() when notifications are turned off - thanks
@canerelci! (#1612)

Support new 833 area code with update to libphonenumber (#1598)

Dev:
  - Support for beta releases installed beside production versions
    (#1606)
  - Display of environment and app instance in title bar/about window
    (#1606)
  - Fix json parsing exceptions obscuring server errors (#1605)
scottnonnenberg added a commit that referenced this pull request Dec 20, 2017
Note: This release is the same thing as https://github.com/WhisperSystems/Signal-Desktop/releases/tag/v1.1.0-beta.6

Listed below are the changes from the previous production release: https://github.com/WhisperSystems/Signal-Desktop/releases/tag/v1.0.41

Update to electron 1.7.9 (#1736)

Support the latest phone number formats via libphonenumber update (#1899)

Reduce download size by ~25MB over the previous production build (#1869)

Emoji - thanks @liliakai:
  - Emoji picker (#1608)
  - Add support for Emoji 5 (#1797)

Notifications:
  - Windows 7: Use an alternate mechanism for notifications (#1812)
  - Prevent drawAttention() when notifications are turned off - thanks @canerelci! (#1612)

Linux:
  - Support for current (artful) and previous (xenial) ubuntu versions (#1856)
  - Fix missing application icon on some Linux distributions (#1735)
  - Fix issue where window would not show new message alerts on some Linux systems - thanks @cornerman (#1820)
  - Add .deb specific dependencies - thanks @veggiedefender (#1858)

The default button is now 'later', not 'restart' in the 'update available' dialog (#1894)

Make the window minimum width a little smaller - thanks @emptyflask (#1863)

Intl-friendly sort order for contact lists (#1900)

Fix issue where update would restore deleted windows shortcut (#1804)

Fix issue where .tif file attachments could not be sent or received (#1901)

(in testing) Add a tray icon to the application behind command-line argument - thanks @m-pilia (#1676)
  --use-tray-icon enables the tray icon
  --start-in-tray enables the tray icon and the application starts minimized in the tray bar

(in testing) Support pass-through proxies with HTTPS_PROXY environment variable (#1878)

Dev:
  - Display of environment and app instance in title bar/about window (#1606)
  - Support for beta releases installed beside production versions (#1606)
  - Fix json parsing exceptions obscuring server errors (#1605)
  - Be resilient to thrown non-errors in import process (#1737)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants