[VBLOCKS-1608] Serialize errors

mhuynh5757 commented 1 year ago

Submission Checklist

[x] ~The CHANGELOG.md reflects any feature, bug fixes, or known issues made in the source code~
[x] New unit tests and integration tests have beed added
[x] Code coverage is more or equal to the previous coverage
[x] A visual inspection of the Files changed tab was made prior to submitting the pull request ensuring the style guide was followed
[x] CI pipeline is green
[x] ~Included screenshot if necessary~

Description

This PR serializes errors using the miniSerializeError function provided by Redux Toolkit. Please note that user actions are not done in this PR, as there is further refactoring work needed as part of VBLOCKS-1834.

Breakdown

Use miniSerializeError on errors before putting them in actions or app state.

Validation

Manually tested outgoing calls.

Additional Notes

Further work will be done to bring user actions up to the same standard.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

[x] I acknowledge that all my contributions will be made under the project's license.

mhuynh5757 commented 1 year ago

Non-serializable values such as error objects should not be put into actions and state as per the Redux team's guidelines. Right now, if an error occurs, the Redux Toolkit middlewares will extensively warn about the error object.

charliesantos commented 1 year ago

Non-serializable values such as error objects should not be put into actions and state as per the Redux team's guidelines. Right now, if an error occurs, the Redux Toolkit middlewares will extensively warn about the error object.

This is not obvious in the code. How do we make sure anyone modifying our code knows this and does not forget?

mhuynh5757 commented 1 year ago

It's a "Best Practice" according to the Redux docs, see here: https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions

Should we make a reference to these docs in our source code somewhere? Not sure what the best solution is.

charliesantos commented 1 year ago

It's a "Best Practice" according to the Redux docs, see here: https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions

Should we make a reference to these docs in our source code somewhere? Not sure what the best solution is.

A comment in the code that has a link to the best practice is better. Readme works too. Since you're doing this in multiple places in the code, is it possible to create a helper/util function that does this that way you can add that comment in a single location?

twilio / twilio-voice-react-native-app