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

feat: Registration/Password lost: better catch 504 errors from the server #777

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

g123k
Copy link
Contributor

@g123k g123k commented Aug 8, 2023

Hi everyone,

Due to the current situation with the infrastructure, we often have 504 errors.
Here is a better catch of the errors for the registration and the password lost.
It's here limited to the user management, but it's more critical here.

@g123k g123k requested a review from a team as a code owner August 8, 2023 09:06
@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2023

Codecov Report

Merging #777 (9022453) into master (dc48c0f) will decrease coverage by 0.16%.
Report is 2 commits behind head on master.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #777      +/-   ##
==========================================
- Coverage   73.16%   73.00%   -0.16%     
==========================================
  Files         203      203              
  Lines        7407     7408       +1     
==========================================
- Hits         5419     5408      -11     
- Misses       1988     2000      +12     
Files Changed Coverage Δ
lib/src/model/sign_up_status.dart 0.00% <ø> (ø)
lib/src/open_food_api_client.dart 49.84% <0.00%> (-0.16%) ⬇️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines +1230 to +1231
/// Trigger real 5xx errors
return status;
Copy link
Member

Choose a reason for hiding this comment

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

Say the status is 502 then this case would never trigger. Everything in the besides 500 Returns a 400

Is that your wanted behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops my bad, I created this PR while discussing and clearly that was not a good idea.

Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

Looks good now 👍🏼

@M123-dev M123-dev merged commit 8cd4a35 into master Aug 8, 2023
4 checks passed
@M123-dev M123-dev deleted the 504_errors branch August 8, 2023 20:26
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

3 participants