This is a simplified version of error handling, without useful error message. Created a task https://linear.app/comm/issue/ENG-3916/better-error-handling to track implementing the proper solution.
Depends on D7883
Details
Details
- Reviewers
bartek kamil inka - Commits
- rCOMM4bfbf92b81c5: [native] Introduce simple error handling
Check if creating a link with white char or one that is already in use results in an error.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/invite-links/manage-public-link-screen.react.js | ||
|---|---|---|
| 44 ↗ | (On Diff #26687) | I would prefer a less generic name to be more descriptive but up to you |
| 53 ↗ | (On Diff #26687) | I don't where this error will be caught and how this will behave, but maybe logging an error message here is a good idea? |
| 68–71 ↗ | (On Diff #26687) | You can use an empty string instead of null in the state (const [error, setError] = React.useState('');), and then render this component always and get rid of this logic, which is shorter and I think also cleaner |
| native/invite-links/manage-public-link-screen.react.js | ||
|---|---|---|
| 44 ↗ | (On Diff #26687) | When there's just one action creator in this component, I think it is ok to be really generic - it's not confusing. Later, in D7908, I'm introducing another action creator and then this name changes to be more precise. |
| 53 ↗ | (On Diff #26687) | There's no need for that. Throwing an error in async function passed to dispatchActionPromise results in appropriate _FAILED action being dispatched. Then the caller can decide how to handle that (using createLoadingStatusSelector). |
| 68–71 ↗ | (On Diff #26687) | The difference is that it would then take vertical space, so it's better to not render the error in that case. |