[lib] Replace cloneError() with SendMessageError
ClosedPublic
Actions

Authored by angelika on Mon, Nov 4, 12:41 PM.

Details

Reviewers

kamil
tomek

Commits

rCOMM715cdf31911a: [lib] Replace cloneError() with SendMessageError

Summary

https://linear.app/comm/issue/ENG-9730/native-crashes-when-call-to-identity-fails

Test Plan

Force the error when sending message:
0. Create DM with some user

Comment out error swallowing in peer-list-hooks.js
Change identity address to some random address
Remove device lists by dispatching an action as described in the issue
Try to send a message
Verify that the app doesn't crash and user can retry sending the message and you can see the error nicely in the logs

Diff Detail

Repository

rCOMM Comm

Branch

graszka22/ENG-9730

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

angelika created this revision.Mon, Nov 4, 12:41 PM

Herald added a subscriber: ashoat. · View Herald TranscriptMon, Nov 4, 12:41 PM

angelika added a child revision: D13863: [lib] Remove unecessary utils-copy-error dependency.Mon, Nov 4, 12:43 PM

Harbormaster completed remote builds in B32520: Diff 45586.Mon, Nov 4, 12:59 PM

angelika requested review of this revision.Mon, Nov 4, 12:59 PM

Can you add a specific message to each SendMessageError error? I realize it wasn't there before, but it might be helpful here to have some additional context

native/chat/reaction-message-utils.js
118–122
native/input/input-state-container.react.js
593	Accidentally left over

kamil added inline comments.Tue, Nov 5, 1:41 AM

native/chat/reaction-message-utils.js
118–122	Looks like this type is not correct, but was not correct in the past either, see here Wondering, if we should use `SendMessageError` type in `/lib/types/redux-types.js` to catch things like this? I am talking about this three specific actions: `SEND_MULTIMEDIA_MESSAGE_FAILED` `SEND_REACTION_MESSAGE_FAILED` `SEND_TEXT_MESSAGE_FAILED`

Review changes

Harbormaster completed remote builds in B32530: Diff 45598.Tue, Nov 5, 4:04 AM

ashoat added inline comments.Tue, Nov 5, 4:34 AM

native/chat/reaction-message-utils.js
118–122	Wondering, if we should use `SendMessageError` type in `/lib/types/redux-types.js` to catch things like this? Good call... yeah, I think that would be best. In addition to the changes @angelika made here to `lib/types/redux-types.js`, I think we should update the place(s) where the error is thrown... looks to be in `lib/hooks/input-state-container-hooks.js`. Hopefully we can remove the `any`-cast there