Promote develop to main: the sync agent coupling, pinned - #866
Merged
Merged
Conversation
The agent declares bson, dotenv and mongodb and ships with none of them. Verified on a real installed app: resources/sync-agent/ holds package.json, src/ and sync-config.json, and no node_modules at all. It works anyway, and the reason is three files apart from each other, none of which mentions the others. server.js puts the API's node_modules on process.env.NODE_PATH, for the API. sync-agent-manager spawns the agent with ...process.env. The agent inherits that and resolves its three packages from the API's copy, which already carries all of them. So cloud sync depends on the API having started first, on a variable set for a different purpose, and on a spread operator. Remove any one and it dies with MODULE_NOT_FOUND on a customer machine and nowhere else. That is not a complaint about the design - a second copy of mongodb in the installer would be real weight for no gain. It is a complaint about the coupling being invisible, which is the shape of every fault found in this app today: correct code silently relying on something nobody could see from where they were standing. Pinned rather than rewired. Cloud sync is load-bearing and a test costs nothing, while changing a working spawn costs a shop's orders if it is wrong. The packaging step also stops calling the normal case a warning. It printed "WARNING: agent copied without node_modules - run npm install in the Cloud agent first" on every correct build, and following that advice would have made every installer larger to fix nothing. A warning that fires when everything is right is one people learn to scroll past, and then the real one scrolls past too.
…les-on-purpose The sync agent finds its modules by a route nobody can see
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, no runtime change. Kept level with
mainso the next exe is built from a branch that matches what has been reviewed.Carries
The sync agent finds its modules by a route nobody can see (#865).
The agent declares
bson,dotenvandmongodband ships with none of them. Verified on the installed app:resources/sync-agent/holdspackage.json,src/andsync-config.jsonand nonode_modules, while the log shows it syncing 91 orders.It works because
src/server.jsputs the API'snode_modulesonNODE_PATHfor the API, andsrc/sync-agent-manager.jsspawns the agent with...process.env. So cloud sync depends on the API starting first, on a variable set for a different purpose, and on a spread operator that reads like boilerplate. Remove any one and it dies withMODULE_NOT_FOUNDon a customer machine and nowhere else.Pinned with four tests rather than rewired: shipping a second copy of mongodb would be dead weight, the design is right, and changing a working spawn mid-handover is not worth it. One of the tests fails if the agent ever grows a dependency the API does not carry, which would otherwise surface as a sync that stopped, at night, on a till.
The packaging step also stops printing a warning on every correct build telling whoever saw it to run
npm installin the Cloud agent, which would have made every installer larger to fix nothing.After this
Re-stage with
npm run buildornpm run publish. Then on the kitchen machine:npm run check:installed, Core Settings, both switches on, press Test the sound.Do not squash.