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.

Sign up for GitHub

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

don't throw IllegalStateException #3495

Closed
wants to merge 1 commit into from
Closed

don't throw IllegalStateException #3495

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/thoughtcrime/securesms/ConversationActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,7 @@ private void initializeResources() {
}
chatId = getIntent().getIntExtra(CHAT_ID_EXTRA, -1);
if(chatId == DcChat.DC_CHAT_NO_CHAT)
throw new IllegalStateException("can't display a conversation for no chat.");
chatId = -1;
Copy link
Member Author

Choose a reason for hiding this comment

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

mmh, actually this seems to be there to prevent from opening the trash chat that happens to be chatId == 0?

Copy link
Member

@r10s r10s Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

nope. the "trash" has id 6 == DC_CHAT_ID_TRASH. however, that is internal and the UI should not care about that

and at comparable places, we're also using getIntExtra(CHAT_ID_EXTRA, DC_CHAT_NO_CHAT)

just saying, not sure under which circumstances this "throw" can happen and what the gist of the check is

Copy link
Member Author

Choose a reason for hiding this comment

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

and at comparable places, we're also using getIntExtra(CHAT_ID_EXTRA, DC_CHAT_NO_CHAT)

mmmh maybe it is a bug but we are using a lot -1 instead of DC_CHAT_NO_CHAT, so in this line code section, if CHAT_ID_EXTRA was not present, it would have been set to -1 and not throwing the exception then

Copy link
Member

Choose a reason for hiding this comment

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

yip, i was just mentioning, it is not an error to deal with -1 as well as 0.

not crashing is probably not that bad, but we could also do finish(); return - however, for that change, it may be good to have it reproducible (not that we're called when the activity is already in finishing or so ..)

why is the PR closed, btw?

Copy link
Member Author

Choose a reason for hiding this comment

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

why is the PR closed, btw?

just that change doesn't solve the problem, I tried passing chatId zero in extras to conversationactivity and then there is no error, but still some weird empty messages were loaded without text, just dates, so it seems in my test account which is not super-old the chatid 0 exists somehow 🤯

dcChat = dcContext.getChat(chatId);
recipient = new Recipient(this, dcChat);
glideRequests = GlideApp.with(this);
Expand Down
Loading