Skip to content

Commit

Permalink
Fix first run config validation and reload
Browse files Browse the repository at this point in the history
Hasty refactoring lead to a broken first run experience due to config validation
being blocked on first run. Unblock that, and also fix a couple other issues:

* When validating path mappings, keep the default if there aren't any user-
  provided mappings, so we don't write empty mappings to the config file.
* Soft-reloading the server after initial setup was broken because the action
  wasn't allowed in RunningWithoutConfig mode.
* The server would be in a broken state if the user's initial setup didn't
  require a server reload (e.g. only changed the log level).
  • Loading branch information
danrahn committed Apr 30, 2024
1 parent a61d26c commit 3f93595
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Client/Script/ServerSettingsDialog/PathMappingsTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export class PathMappingsTable {
}

return {
value : newMappings,
value : newMappings.length === 0 ? null : newMappings,
defaultValue : [],
isInvalid : false,
};
Expand Down
12 changes: 11 additions & 1 deletion Server/Commands/ConfigCommands.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ServerConfigState } from '../../Shared/ServerConfig.js';

import { ServerEvents, waitForServerEvent } from '../ServerEvents.js';
import { ServerState, SetServerState } from '../ServerState.js';
import { Config } from '../MarkerEditorConfig.js';
import { PostCommands } from '../../Shared/PostCommands.js';
import { registerCommand } from './PostCommand.js';
Expand Down Expand Up @@ -47,6 +48,7 @@ async function validateConfigValue(setting, value) {
* @param {SerializedConfig} config
* @param {ServerResponse} response */
async function setConfig(config, response) {
const oldConfigState = Config.getValid();
const newConfig = await Config.trySetConfig(config);
switch (newConfig.config.state) {
case ServerConfigState.FullReloadNeeded:
Expand All @@ -56,7 +58,15 @@ async function setConfig(config, response) {
await waitForServerEvent(ServerEvents.SoftRestart, response, newConfig);
break;
default:
sendJsonSuccess(response, newConfig);
// If we were previously in a valid state, we can just mark the server as
// running and return. Otherwise, we need to do a reload, since an invalid
// state implies we haven't set up any of our core classes.
if (oldConfigState === ServerConfigState.Valid) {
SetServerState(ServerState.Running);
sendJsonSuccess(response, newConfig);
} else {
await waitForServerEvent(ServerEvents.SoftRestart, response, newConfig);
}
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion Server/MarkerEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ function userResume(res) {
* @param {ServerResponse} res */
async function userReload(res, data) {
Log.verbose('Attempting to reload marker data');
if (GetServerState() !== ServerState.Running) {
if (![ServerState.Running, ServerState.RunningWithoutConfig].includes(GetServerState())) {
return sendJsonError(res, new ServerError('Server must be running in order to reload.', 400));
}

Expand Down
2 changes: 1 addition & 1 deletion Server/PostCommands.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import ServerError from './ServerError.js';
const Log = new ContextualLog('POSTCommands');

/** Subset of server commands that we accept when the config files doesn't exist or is in a bad state. */
const badStateWhitelist = new Set(['get_config', 'valid_config', 'valid_cfg_v', 'set_config']);
const badStateWhitelist = new Set(['get_config', 'validate_config', 'valid_cfg_v', 'set_config']);

/**
* Verify that the given endpoint is allowed given the current server state.
Expand Down

0 comments on commit 3f93595

Please sign in to comment.