Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upCheck text on bad request status #290
Conversation
AlexeyBarabash
force-pushed the
status_400_staging
branch
2 times, most recently
from
568d957
to
9a40707
|
++ |
| if (response.status === 400) { | ||
| // Bad request | ||
| return response.text().then((text) => { | ||
| if (text === 'Signed request body of the client timestamp is required.') { |
This comment has been minimized.
This comment has been minimized.
darkdh
Mar 26, 2019
Member
Why can't we just throw new Error(`Credential server response ${response.status}: ${text}`) when not ok? Response text can be changed from time to time
This comment has been minimized.
This comment has been minimized.
AlexeyBarabash
Mar 26, 2019
Author
Contributor
I thought we cannot trust the data from the network :) . But with your question, I understand this is our server, so we can. Thanks.
This comment has been minimized.
This comment has been minimized.
|
Hold on with review, just found the case while test this PR with brave/brave-core#2076: UI thread can be blocked and browser becomes unresponsive if I am trying to enable sync and wait too long. |
|
I could not reproduce the mentioned situation, but created the issue brave/brave-browser#3954 . @darkdh Could you please re-review. Linked brave-core PR brave/brave-core#2076 . |
Check text on bad request status
AlexeyBarabash commentedMar 22, 2019
•
edited
When the time or/and time zone on the client device is not correct, sync auth server rejects authentication with error 400/'Signed request body of the client timestamp is required.'. The clients can see only
Credential server response 400. For the case when it is caused by time issue, this is passed to the client.Fixes: #157 .
Probably addresses:
#114
#212
brave/browser-android-tabs#788