Broken room due to upgrade shenanigans

[babolivier]

First part of my adventures in messing up room upgrades is here: matrix-org/synapse#4583

Since I left the wrong room, I tried to go and redact the m.room.tombstone event (which worked). Because bad luck never stops that early, I deleted the wrong one, i.e. the one linking to the room I still was in.
So I tried re-accessing the room to see if I could fix the situation, and Riot just refused to display it. In the console was this error:

Warning: Exception thrown by hook while handling onSetChildren: Invariant Violation: Expected onMountComponent() to fire for the child before its parent includes it in onSetChildren().
invariant@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:318379:15
onSetChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:353544:59
callHook@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:343665:5
emitEvent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:343677:7
onSetChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:343949:5
setChildrenForInstrumentation@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:345745:7
_updateChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:345946:9
updateChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:345885:7
_updateDOMChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:341515:7
updateComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:341329:5
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:341291:5
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347068:5
_updateRenderedComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340295:7
_performComponentUpdate@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340265:5
updateComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340186:7
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340088:5
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347068:5
updateChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:339401:9
_reconcilerUpdateChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:345794:11
_updateChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:345898:26
updateChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:345885:7
_updateDOMChildren@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:341515:7
updateComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:341329:5
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:341291:5
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347068:5
_updateRenderedComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340295:7
_performComponentUpdate@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340265:5
updateComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340186:7
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340088:5
receiveComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347068:5
_updateRenderedComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340295:7
_performComponentUpdate@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340265:5
updateComponent@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340186:7
performUpdateIfNecessary@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:340102:7
performUpdateIfNecessary@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347100:5
runBatchedUpdates@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347980:5
perform@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:349941:13
perform@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:349941:13
perform@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347919:12
flushBatchedUpdates@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:348002:7
closeAll@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:350007:11
perform@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:349954:11
batchedUpdates@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:344069:14
enqueueUpdate@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:348030:5
enqueueUpdate@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347612:3
enqueueSetState@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347806:5
./node_modules/react/lib/ReactBaseClasses.js/ReactComponent.prototype.setState@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:353079:3
_onWillStartClient/<@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:74276:13
./node_modules/events/events.js/EventEmitter.prototype.emit@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:316369:7
./matrix-js-sdk/lib/sync.js/</SyncApi.prototype._updateSyncState@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:35146:5
_callee7$@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:34164:29
tryCatch@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:49715:37
invoke@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:49949:22
defineIteratorMethods/</prototype[method]@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:49767:16
tryCatcher@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:43536:16
./matrix-js-sdk/node_modules/bluebird/js/browser/bluebird.js/</</<[16]</module.exports/PromiseSpawn.prototype._promiseFulfilled@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:40669:18
./matrix-js-sdk/node_modules/bluebird/js/browser/bluebird.js/</</<[22]</module.exports/Promise.prototype._settlePromise@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:41739:17
./matrix-js-sdk/node_modules/bluebird/js/browser/bluebird.js/</</<[22]</module.exports/Promise.prototype._settlePromise0@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:41779:5
./matrix-js-sdk/node_modules/bluebird/js/browser/bluebird.js/</</<[22]</module.exports/Promise.prototype._settlePromises@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:41859:13
_drainQueueStep@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:38772:9
_drainQueue@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:38765:9
./matrix-js-sdk/node_modules/bluebird/js/browser/bluebird.js/</</<[2]</Async.prototype._drainQueues@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:38781:5
Async/this.drainQueues@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:38651:9
run@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:310291:22
notify/<@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:310308:30
flush@https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:306109:9

With a bunch of TypeError: null has no properties with this stacktrace:

getClosestInstanceFromNode   ReactDOMComponentTree.js:113
findParent                   https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:344407:10
handleTopLevelImpl           https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:344436:28
perform                      Transaction.js:143:5
batchedUpdates               ReactDefaultBatchingStrategy.js:62
batchedUpdates               https://riot.im/develop/bundles/f8ff1fbb89bab9ca7ebb/bundle.js:347927:10
dispatchEvent                ReactEventListener.js:147
dispatchEvent                self-hosted:1018:17 

[lampholder]

I'm P2 minoring this since it sounds like you have to fair vandalise the room to get the error - if this is likely to be a common occurance maybe we should spend time making riot handle it better.

[lampholder]

On reflection, this sort of vandalisation (accidental) is probably going to happen - it's probably just highlighted some assumptions in Riot web we shouldn't be making.

If this is something that users can do by mistake (or on purpose) as part of regualr operation, we should probably fix it before room upgrades are commonplace.

[turt2live]

ftr, rageshakes would be super helpful for stuff like this to help narrow down any issues that could have lead up to the obvious stack trace.

Although, I think the pieces required to reproduce this are patched away into oblivion:

• Use leaveRoomChain when leaving a room matrix-org/matrix-react-sdk#2818 fixes leaving the wrong room by leaving all of them (probably unhelpful rather than helpful here, though)
• Don't upgrade a room if there's already an ongoing upgrade matrix-org/synapse#4583 would indeed fix this from happening again
• https://github.com/matrix-org/matrix-doc/issues/1937 would never let this happen in the first place

With that being said though, I'll try and reproduce the elements of it which cause Riot to explode and see what happens.