Skip to content

Conversation

dab246
Copy link
Member

@dab246 dab246 commented Oct 7, 2025

Issue

#4081

Analysis and Solutions

Error handling

  • When encountering a network connection error (NoNetworkError), display the message "No internet connection" to the user.
  • When encountering a 401 error, perform a token refresh (refreshToken).
    • If refreshToken throws an exception, display a message to the user in the format:
      "Message [CodeError1 - CodeError2]".
    • Otherwise, if refreshToken succeeds but returns an empty token, automatically log in again using the current user's information.

Demo

  • Case: Auto re-login with user email on 401 error for mobile
auto-refresh.webm
  • Case: User logs out
logout.webm

Copy link

github-actions bot commented Oct 7, 2025

This PR has been deployed to https://linagora.github.io/tmail-flutter/4088.

@chibenwa
Copy link
Member

chibenwa commented Oct 7, 2025

Auto re-login with user email on 401 error for mobile

It could be great to display somehow an explanation of what happens to the user.
I think it could reduce the friction

Copy link
Member

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

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

TOO LONG, DID NOT READ

1000+ LOC changeset is a lot, even more for a simple bugfic.

I see a lot of duplicated try catch.

This is IMO symptomatic. I would have expected either a generic behaviour of the HTTP library, or an abstraction lying in tmail-flutter or even better in jmap-dart to help us significantly reduce the boiler plate code.

@dab246
Copy link
Member Author

dab246 commented Oct 8, 2025

TOO LONG, DID NOT READ

1000+ LOC changeset is a lot, even more for a simple bugfic.

I see a lot of duplicated try catch.

This is IMO symptomatic. I would have expected either a generic behaviour of the HTTP library, or an abstraction lying in tmail-flutter or even better in jmap-dart to help us significantly reduce the boiler plate code.

IMO, this is not a simple bugfix.
Let me explain why there are so many duplicated try-catch blocks.

  • I have separated the commits clearly for each specific bugfix.

  • The reason for the 1000+ changed lines is due to the last commit, which addresses the following runtime error related to the Future.catchError method:

Invalid argument(s) (onError): The error handler of Future.catchError must return a value of the future's type

To fix this issue properly, we needed to update all related data sources. Since we are following Clean Architecture, with clear separation and abstraction across layers, multiple features depend on different data sources. Therefore, we have to apply try/catch in multiple places. However, the exception handling logic itself is centralized in a single common class, ensuring consistency across the app.

@dab246
Copy link
Member Author

dab246 commented Oct 8, 2025

Auto re-login with user email on 401 error for mobile

It could be great to display somehow an explanation of what happens to the user. I think it could reduce the friction

If you have any suggestions, please share them with us.
It would be even better if you could provide a mockup or a video.

@dab246 dab246 requested a review from chibenwa October 8, 2025 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants