we want to introduce this component now so that we can check in the next web release if the production identity service is reachable. before this change, ashoat's keyserver was the only device that called identity. going forward, all web clients should be connecting to identity as well.
Details
Details
- Reviewers
ashoat - Commits
- rCOMMc82807eb9cb6: [web] introduce component to ping identity
saw "Identity ping successful" log in browser console
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- webping
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Can you link to the Linear task? Wondering if we're planning to do this on native as well
| web/components/identity-ping.react.js | ||
|---|---|---|
| 8 | I would move this definition down closer to where it's used | |
| 24 | I don't love that this gets printed in production for normal users. Could we check useIsCurrentUserStaff here? | |
| 30–36 | This is a good technique for making sure we only call ping once per page load. But if we take my advice above, I think that also would mean that we wouldn't print it unless the user is logged in at the time the page is loaded. Techniques to get around that include:
Option 4 might be easiest here, but it would be good to check and make sure it doesn't lead to endless log spam. | |
| web/grpc/identity-service-client-wrapper.js | ||
| 671–674 | Minor, but you can avoid async/await syntax here, and also make this a little shorter | |
Comment Actions
https://linear.app/comm/issue/ENG-8138/add-a-ping-call-to-web
I should've clarified that the purpose of this diff is to make sure that identity can be reached via gRPC-web in production
tested that there isn't any log spam after removing hasRun
| web/grpc/identity-service-client-wrapper.js | ||
|---|---|---|
| 671–674 | this actually doesn't work because this.unauthClient.ping(new Empty()) returns Promise<Empty> i'm going to revert this change | |