Details
Manual testing with mocked invalid access token
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
reassignThickThreadMediaForThinThread does not appear to be handled, even though it was extracted in D13959.
I'm worried that we're on relying on diff reviewers to do a thorough analysis of all callsites, which is not an appropriate role for a reviewer.
@bartek, if I missed it, can you please link the diff where this is handled? On the other hand, if it's not handled yet, can you share how you're keeping track of each callsite that needs to be migrated?
| web/input/input-state-container.react.js | ||
|---|---|---|
| 1224–1230 ↗ | (On Diff #45977) | Why is there no equivalent call on native? |
| web/input/input-state-container.react.js | ||
|---|---|---|
| 1224–1230 ↗ | (On Diff #45977) | On web, we upload media before sending a message and user can cancel the upload. This is the code for removing holder of canceled upload. |
| web/input/input-state-container.react.js | ||
|---|---|---|
| 1224–1230 ↗ | (On Diff #45977) | Thanks for the explanation, that makes sense |
Thanks for handling reassignThickThreadMediaForThinThread !
Would mind sharing how you're keeping track of each callsite that needs to be migrated? I'm worried because I caught two cases in my review, but I didn't do an exhaustive analysis, and so I'm worried there are other cases I might not have caught.
I used 'git grep'-like vim plugin to find usages of:
- Blob service endpoints from lib/facts/blob-service.js. Extracted these to separate functions
- Repeated for call sites of these functions.
- If the callsite re-throws the invalid_csat, found the try-catch block that wraps it and added handling there.
For callsites that you found:
- Two of them were deliberately omitted e.g. because they were called during logout
- For one I understood the code badly (keyserver override from D13989)
- This one - here I missed that the exception is thrown in two different functions. Now I repeated step 3 above to make sure I didn't miss more calls (git grep invalid_csat)
I'm going to repeat the above steps before landing and scratch a kind of call stack tree of all Blob calls
For Tunnelbroker / Identity search:
- git grep for CONNECTION_INITIALIZATION_RESPONSE
For Identity RPCs:
- native - they're all grouped in Identity Context. Looked which ones are in auth proto and wrapped them in D14024.
- web - due to shared worker architecture, I intercepted the common place for all RPC calls in D14025.
- keyserver - git grep for getRustAPI() - extracted RPC calls from auth proto to separate functions and wrapped them in D14029
Reports: endpoint is optionally-authenticated (just to send user ID if present) so nothing to do
Feature flags: endpoint is unauthenticated
Backup: I had the draft locally, but the implementation for performing backup is ongoing significant changes, discussed this offline with @kamil, and he is going to do this when working on ENG-9604.