Reviewable / Reviewable

Support for Reviewable
https://reviewable.io
139 stars 30 forks source link

Reviewable gets stuck waiting on permission checks #819

Closed pkaminski closed 2 years ago

pkaminski commented 3 years ago

Reviewable will sometimes get stuck waiting on permission checks, especially when first loading the Reviews page. The issue appears to be client-side: either we're not creating permission tickets, or we don't notice when they're fulfilled. This is hard to debug since I can't get a consistent repro, even though it happens often enough to be annoying.

mtlynch commented 2 years ago

I can put in some extra instrumentation and give you some code to run in the JS console next time it happens -- perhaps it can help me snag a clue to unravel the mystery.

Sure, let me know when you're ready for me to run the JS code.

pkaminski commented 2 years ago

Cool! Next time you get stuck, please run this and let me know the output (send to support@reviewable.io if sensitive):

JSON.stringify(_(truss.store.services.warden.permissions).reject('verdictAcquired').keyBy('$key').mapValues(p => ({timestamp: p.ticketRef.child('timestamp').value, writeTimestamp: p.ticketRef.child('writeTimestamp').value})).value())

This will hopefully let me figure out whether the permission tickets are actually being written to the server, and if so whether the failure is in processing them or in receiving the results on the client. Thanks!

mtlynch commented 2 years ago

Caught a repro today:

JSON.stringify(_(truss.store.services.warden.permissions).reject('verdictAcquired').keyBy('$key').mapValues(p => ({timestamp: p.ticketRef.child('timestamp').value, writeTimestamp: p.ticketRef.child('writeTimestamp').value})).value())
'{"raspberrypisig|pizero-usb-hid-keyboard|github:7783288":{"timestamp":1657317961935,"writeTimestamp":1657317961706},"mtlynch|rebalancer|github:7783288":{"timestamp":1657317961941,"writeTimestamp":1657317961708},"mtlynch|docker-godot|github:7783288":{"timestamp":1657317961964,"writeTimestamp":1657317961716},"mtlynch|refactoring-english-landing|github:7783288":{"timestamp":1657317961983,"writeTimestamp":1657317961723},"mtlynch|whatgotdone|github:7783288":{"timestamp":1657317961998,"writeTimestamp":1657317961729},"mtlynch|wanderjest|github:7783288":{"timestamp":1657317962019,"writeTimestamp":1657317961735},"tiny-pilot|ansible-role-tinypilot|github:7783288":{"timestamp":1657317962025,"writeTimestamp":1657317961736},"tiny-pilot|gatekeeper|github:7783288":{"timestamp":1657317962031,"writeTimestamp":1657317961737},"mtlynch|gridsome.org|github:7783288":{"timestamp":1657317962064,"writeTimestamp":1657317961747},"mtlynch|mtlynch.io|github:7783288":{"timestamp":1657317962069,"writeTimestamp":1657317961754}}'
pkaminski commented 2 years ago

Thanks -- the data points towards the server not processing the permission tickets correctly. Did you reload the page immediately afterwards (presumably without error), or did you leave things as-is for a while?

mtlynch commented 2 years ago

No, I didn't reload or interact with Reviewable since collecting the debug output.

pkaminski commented 2 years ago

Quick update: while I haven't reproduced this exact issue, I did finally isolate a related Firebase data integrity bug that may be the root cause here as well. It's been escalated to Google engineering a couple days ago and I can only hope they get to it soon...

pkaminski commented 2 years ago

So the Firebase I isolated got fixed, but it didn't affect the wider behavior I was hoping it would. On the other hand, I got another report of the issue and managed to confirm my suspicions in a timely manner, which led me to releasing a workaround a couple days ago. It's not perfect, but I believe it should mitigate the issue (at the cost of some additional latency) until I manage to get Firebase to fix the root cause.