don't throw IllegalStateException #3495
Conversation
|
To test the changes in this pull request, install this apk: |
| @@ -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; | |||
There was a problem hiding this comment.
mmh, actually this seems to be there to prevent from opening the trash chat that happens to be chatId == 0?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 🤯
the problem is that
CHAT_ID_EXTRApassed toConversationActivityis0in the reported issue, I looked at several places and often it is checked thatchatId !=0etc. so I couldn't really find a place in which it could be zero, so instead of a crash I changed it to be handle similar to other situations where the chatId is invalid or non-existing chatclose #3494