search

iyu46 / RemovedConnectionAlerts

Keep track of your friends and servers on BetterDiscord, and get notified if you get unfriended or kicked from a server.
11 stars 6 forks source link

Crashing when changing channels #3

Closed pallen0304 closed 1 year ago

pallen0304 commented 1 year ago

Looks like a race condition, the channelHeaderInboxIcon and channelheaderInboxIcon aren't present when the code is fired to add the Recents button (i.e. insertButtonAtLocationWithStyle).

These are the errors I get: image image

The issue doesn't occur and the plugin functions for me after making changes in PR #2

pallen0304 commented 1 year ago

This may be an issue that only occurs as a collision with other plugins, but these 2 added null checks stop my Discord from crashing when navigating to any Server or swapping channels, and the plugin continues to function normally.

This suggests it may be executing more often than it needs to though if it's calling it too early and still getting called later.

iyu46 commented 1 year ago

Thank you so much, that's absolutely fair! It does make sense that making the icons nullable would stop crashes, though I figure that there's not much that can be done modifying the observer callback since Discord lazy-loads their elements, and if an element isn't there it's either 1. really not there or 2. the elements they attach to just haven't loaded in yet - while this helps in the first case I haven't figured out a non-blocking solution to the second. (Another BD plugin dev mentioned to me how they check whether or not to place their buttons and it involves checking the number of removed/added elements on the screen against a static numerical threshold, which I didn't implement myself because the magic number added uncertainty)

When triggering this nullable to happen, does the observer callback still fire later on? Like does the icon still show up even after this error happens (and it gets guarded by the nullable)?

iyu46 commented 1 year ago

omg i just ran into when this is happening. wow this is really annoying why did discord do this to me LOL i'm going to merge it in and then figure out more logistics after

pallen0304 commented 1 year ago

When triggering this nullable to happen, does the observer callback still fire later on? Like does the icon still show up even after this error happens (and it gets guarded by the nullable)?

It's getting triggered more than once, but if Discord crashes, no. If Discord doesn't crash, it adds it on any view that includes the Inbox icon as original behavior. This was the experience I had. In detail, with my changes, channelHeaderInboxIcon?.parentElement.insertBefore, if channelHeaderInboxIcon is NULL/undefined/falsy, then the rest of the code in that statement is ignored, which means insertBefore is never called, so no icon is added. -- In retrospect, this method could be simplified to 'failfast', but I was trying to fix it before I started my work day so I could have Discord up 😄

The other change..impacts the same button we're inserting? If help isn't present, rcaModalBtn doesn't get the "button" class defined in Constants.. not sure what's in that CSS Class, couldn't find "button-1fGHAH" in my Discord.

iyu46 commented 1 year ago

Thanks! yepyep, I understood your changes :p

For the "button-1fGHAH" class, the buttons in the toolbar tray are styled differently when you have a voice channel/popout open! So that style gets applied at that time, it's not normally present out of call.

I can close this issue if there's no further problems? It seems that I'm not running into any crashing issues anymore either :00

pallen0304 commented 1 year ago

I agree that this issue with this plugin is resolved. Thanks for creating and actively maintaining this repo!