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

ui: Incorrect null-check operator in _LoadingPlaceholderPageState.build #1219

Open
chrisbobbe opened this issue Dec 27, 2024 · 1 comment · May be fixed by #1235
Open

ui: Incorrect null-check operator in _LoadingPlaceholderPageState.build #1219

chrisbobbe opened this issue Dec 27, 2024 · 1 comment · May be fixed by #1235
Assignees
Milestone

Comments

@chrisbobbe
Copy link
Collaborator

To reproduce:

  1. Use a debug build so you can see errors printed to the console
  2. Log into an account
  3. Log out of the account ("Choose account" > three-dots menu > "Log out")
  4. See the following in the console:
======== Exception caught by widgets library =======================================================
The following _TypeError was thrown building _LoadingPlaceholderPage(dirty, dependencies: [_GlobalStoreInheritedWidget, _LocalizationsScope-[GlobalKey#c7674]], state: _LoadingPlaceholderPageState#4549b):
Null check operator used on a null value

The relevant error-causing widget was: 
  _LoadingPlaceholderPage _LoadingPlaceholderPage:file:///Users/chrisbobbe/dev/zulip-flutter/lib/widgets/home.dart:36:31
When the exception was thrown, this was the stack: 
#0      _LoadingPlaceholderPageState.build (package:zulip/widgets/home.dart:184:36)
#1      StatefulElement.build (package:flutter/src/widgets/framework.dart:5841:27)
#2      ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:5733:15)
#3      StatefulElement.performRebuild (package:flutter/src/widgets/framework.dart:5892:11)
#4      Element.rebuild (package:flutter/src/widgets/framework.dart:5445:7)
#5      ComponentElement._firstBuild (package:flutter/src/widgets/framework.dart:5715:5)
#6      StatefulElement._firstBuild (package:flutter/src/widgets/framework.dart:5883:11)
#7      ComponentElement.mount (package:flutter/src/widgets/framework.dart:5709:5)

This is a latent bug introduced in a97ac42 (fyi @PIG208). This widget shouldn't assume that an account for its accountId param exists; in particular, it won't exist in the short period after the account is removed from the database (for logout) and before routeToRemoveOnLogout is processed in a post-frame callback.

I see two plausible fixes:

  • Remove the accountId param and pass…hmm, I guess a realmUrl param, for the "try another account" message. Or:
  • Keep the accountId param but give it dartdoc with a reminder that the corresponding account might not exist, saying why.

In either case we should add a test that would fail before the fix but passes after it.

@chrisbobbe
Copy link
Collaborator Author

Tentatively assigning @PIG208 as the author of a97ac42.

PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Dec 30, 2024
We could pass realmUrl when initializing the `_LoadingPlaceholderPage`,
but that will still require a check for the existence of the account.

The loading page will be blank when the account does not exist. The user
can't easily reach this page because they can only logout from
`ChooseAccountPage`, until we start invalidating API keys. Even then,
they will only see the blank page briefly before getting navigated, so
we should not show any text at all.

Fixes: zulip#1219

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jan 2, 2025
We could pass realmUrl when initializing the `_LoadingPlaceholderPage`,
but that will still require a check for the existence of the account.

The loading page will be blank when the account does not exist. The user
can't easily reach this page because they can only logout from
`ChooseAccountPage`, until we start invalidating API keys. Even then,
they will only see the blank page briefly before getting navigated, so
we should not show any text at all.

Fixes: zulip#1219

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jan 2, 2025
We could pass realmUrl when initializing the `_LoadingPlaceholderPage`,
but that will still require a check for the existence of the account.

The loading page will be blank when the account does not exist. The user
can't easily reach this page because they can only logout from
`ChooseAccountPage`, until we start invalidating API keys. Even then,
they will only see the blank page briefly before getting navigated, so
we should not show any text at all.

Fixes: zulip#1219

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jan 2, 2025
We could pass realmUrl when initializing the `_LoadingPlaceholderPage`,
but that will still require a check for the existence of the account.

The loading page will be blank when the account does not exist. The user
can't easily reach this page because they can only logout from
`ChooseAccountPage`, until we start invalidating API keys. Even then,
they will only see the blank page briefly before getting navigated, so
we should not show any text at all.

Fixes: zulip#1219

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jan 3, 2025
We could pass realmUrl when initializing the `_LoadingPlaceholderPage`,
but that will still require a check for the existence of the account.

The loading page will be blank when the account does not exist. The user
can't easily reach this page because they can only logout from
`ChooseAccountPage`, until we start invalidating API keys. Even then,
they will only see the blank page briefly before getting navigated, so
we should not show any text at all.

Fixes: zulip#1219

Signed-off-by: Zixuan James Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants