Details
Call commCoreModule.generateRandomString.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
---|---|---|
848 | Is CommCoreModule the best place for this? I vaguely recall that we had been separating some stuff out into other files |
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
---|---|---|
848 | Nit: The size argument should be integer. | |
848 |
I recall a conversation with @kamil where he shared similar concerns about over-populating CommCoreModule and planned to refactor the code if there iso enough time at the end of the month. The reason for not doing ot now is that it is not so straightforward since there are already some methods in CommCoreModule like getUserPublicKeys that utilize cryptoThread and are called from JS. So we either have to:
| |
857 | I am afraid that this might not be the right way of checking for exception here. I have recently experienced it myself that exception thrown from purely Objective-C iOS code (@throw [NSException....]) is not caught within C++ try{}...catch{}. I am not sure about Java. Since generateRandomString uses native secureRandomBytes we should check that throwing an exception from Objective C and Java is detected here. @kamil could you please check before landing if we are able to catch Objective-C or Java exception here by using try{} .. catch{}? If not then please create a follow up task and assign it to me. |
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
---|---|---|
848 | Thanks @marcin for replying - cryptoThread is the only reason it is here, not in CommUtilsModule or another place.
I don't think so, number is represented as double - check generated files. | |
857 | You're right C++ can't catch NSException - but it should be fine the way it is. iOS: Android: |