-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unified push endpoint: do not fallback to default endpoint in case of failure and add troubleshoot test. #3388
Conversation
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Quality Gate passedIssues Measures |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3388 +/- ##
===========================================
+ Coverage 82.59% 82.63% +0.04%
===========================================
Files 1693 1696 +3
Lines 39646 39690 +44
Branches 4822 4825 +3
===========================================
+ Hits 32745 32798 +53
+ Misses 5216 5207 -9
Partials 1685 1685 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Hi, I was a bit surprised because only some provider have one. The matrix gateway and the discovery request is not part of the UnifiedPush specs, but we did it to help self-hosting the matrix gateway. Hopefully, the matrix gateway will not be required some time, when matrix will support web push pushkind (MSC4174) For instance, Conversation.im is a known distributor that doesn't have any matrix gateway, and it seems someone opened an issue for that: #3571 In a second step, it may makes sense to move to matrix.org's gateway as a fallback (see #3185), and use sygnal's web push pusher. The new UnifiedPush's library generate web push keys and decrypt the push messages. That way it will make the transition to MSC4174 moving from BTW, do you know how to advocate for MSCs ? MSC4173 would be very useful too. PS: The new lib will also fix the error raised when EAX tries to start a foreground service from the background when it doesn't have the right to (#3031 (comment)) once supported by distributors |
Content
When the UnifiedPush endpoint is not detected as a valid Matrix gateway, the application was using a default endpoint.
Now, the endpoint is still checked, but is used even if it is not valid. This allow to use the provided endpoint even if an unexpected error occurs (network error, SSL certificat error, etc.)
A test is added to the troubleshoot test suite to check again the endpoint and alert the user in case of issue. For instance here, the test is failing:
The second commit is adding more unit test on the other troubleshoot tests.
Motivation and context
Make UnifiedPush solution work in more cases and environments.
Screenshots / GIFs
Tests
Tested devices
Checklist