Promote develop to main: every IPC handler has a door - #870
Merged
Merged
Conversation
The most expensive failures this week were not broken code. They were finished code nothing could reach. The kitchen announcement had two real switches and no UI, so the only way to turn it on was typing into a developer console. Nobody did, and a complete feature sat switched off for its whole life. The handset's sold-out long press called POSNIC.askRunOut(), which did not exist, while the till endpoint it would have reached sat live and uncalled. And main.js handled 'open-hardware-manager' with a comment saying it was for a renderer, while preload.js never exposed it - context isolation means a page has no ipcRenderer of its own, so no renderer could ever send it. All three read as finished from the inside. Nothing failed, nothing logged, and the tests were green, because each half was correct on its own. preload.js is the only door between a page and the main process, so a handler it does not expose is a handler nothing can reach. That is checkable, so this checks it, in both directions: a handler no page can send, and a channel the preload offers that nothing in src/ answers. The second is the quieter one - invoke on a missing handler rejects, send goes nowhere at all. The dead handler is removed. Nothing was lost: the Hardware Manager is reachable from the application menu, the settings deep-link target and the tray, all of which call openHardwareManager() directly. NOT_FOR_A_PAGE starts empty on purpose. A name added there is a promise that something which is not a renderer sends it, and the reason belongs beside the name.
Every IPC handler has a door somebody can open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits, keeping
mainlevel with what has been reviewed.Carries
Every IPC handler has a door somebody can open (#869).
main.jshandledopen-hardware-managerwith a comment saying it was for a renderer, andpreload.jsnever exposed it. Context isolation means a page has noipcRendererof its own, so no renderer could ever send it.Nothing is lost by removing it: the Hardware Manager is reachable from the application menu, the settings deep-link target and the tray.
The rule it ships with is the point. This was the third instance in two days of finished code nothing could reach, after the kitchen announcement having two real switches and no UI, and the handset's sold-out long press calling a function that did not exist while its endpoint sat live and uncalled. All three read as finished from the inside, with green tests, because each half was correct on its own.
The test asserts both directions: no handler nothing can reach, and no preload channel nothing answers. The second is the quieter one, since
sendto a missing handler goes nowhere at all and the page waits for something that will never happen.No behaviour changes for a shop.
Do not squash.