WalletConnect / a2

An Asynchronous Apple Push Notification (apns2) Client for Rust

MIT License

136 stars 47 forks source link

Resolve unwrap in build body and when acquiring RwLock #79

Closed threema-donat closed 2 months ago

threema-donat commented 2 months ago

Description

The current version does panic if a RwLock is poisoned and also when a HTTP request could not be constructed from the given user data.

This PR replaces the RwLocks with the ones from parking_lot that do not have poison errors and adds propagation of errors happening when building the http request.

Resolves #69

How Has This Been Tested?

A test has been written for the invalid payload.

Due Dilligence

[x] Breaking change
[ ] Requires a documentation update
[ ] Requires a e2e/integration test update

chris13524 commented 2 months ago

Curious: why not use tokio's RwLock which also does not return Result? Looks like some functions are not currently async but perhaps they could be made such.

threema-donat commented 2 months ago

Curious: why not use tokio's RwLock which also does not return Result? Looks like some functions are not currently async but perhaps they could be made such.

The maintainers of tokio itself say that one should not use the async version of Mutex / RwLock without having reason for that: link. The same can very likely also be said about RwLock.

For this reason, I think it's better to use parking_lot.

chris13524 commented 2 months ago

Please resolve conflicts :)