Merge pull request #1342 from balena-io/fix-call-stack-exceeding

Use safeStateClone to avoid call-stack exceeding errors
This commit is contained in:
CameronDiver 2020-05-21 12:30:18 -04:00 committed by GitHub
commit e5f53d1a2d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 62 additions and 42 deletions

View File

@ -1,5 +1,6 @@
import ApplicationManager from '../application-manager';
import { Service } from '../compose/service';
import { InstancedDeviceState } from '../types/state';
export interface ServiceAction {
action: string;
@ -28,3 +29,10 @@ declare function serviceAction(
target?: Service,
options?: any,
): ServiceAction;
declare function safeStateClone(
targetState: InstancedDeviceState,
// Use an any here, because it's not an InstancedDeviceState,
// and it's also not the exact same type as the API serves from
// state endpoint (more details in the function)
): Dictionary<any>;

View File

@ -10,7 +10,8 @@ export function doRestart(applications, appId, force) {
const app = currentState.local.apps[appId];
const imageIds = _.map(app.services, 'imageId');
applications.clearTargetVolatileForServices(imageIds);
const stoppedApp = _.cloneDeep(app);
const stoppedApp = safeAppClone(app);
stoppedApp.services = [];
currentState.local.apps[appId] = stoppedApp;
return deviceState
@ -43,7 +44,8 @@ export function doPurge(applications, appId, force) {
if (app == null) {
throw new Error(appNotFoundMessage);
}
const purgedApp = _.cloneDeep(app);
const purgedApp = safeAppClone(app);
purgedApp.services = [];
purgedApp.volumes = {};
currentState.local.apps[appId] = purgedApp;
@ -88,3 +90,51 @@ export function serviceAction(action, serviceId, current, target, options) {
}
return { action, serviceId, current, target, options };
}
export function safeStateClone(targetState) {
// We avoid using cloneDeep here, as the class
// instances can cause a maximum call stack exceeded
// error
// TODO: This should really return the config as it
// is returned from the api, but currently that's not
// the easiest thing due to the way they are stored and
// retrieved from the db - when all of the application
// manager is strongly typed, revisit this. The best
// thing to do would be to represent the input with
// io-ts and make sure the below conforms to it
const cloned = {
local: {
config: {},
},
dependent: {
config: {},
},
};
if (targetState.local != null) {
cloned.local = {
name: targetState.local.name,
config: _.cloneDeep(targetState.local.config),
apps: _.mapValues(targetState.local.apps, safeAppClone),
};
}
if (targetState.dependent != null) {
cloned.dependent = _.cloneDeep(targetState.dependent);
}
return cloned;
}
export function safeAppClone(app) {
return {
appId: app.appId,
name: app.name,
commit: app.commit,
releaseId: app.releaseId,
services: _.map(app.services, (s) => s.toComposeObject()),
volumes: _.mapValues(app.volumes, (v) => v.toComposeObject()),
networks: _.mapValues(app.networks, (n) => n.toComposeObject()),
};
}

View File

@ -15,7 +15,7 @@ import log from '../lib/supervisor-console';
import supervisorVersion = require('../lib/supervisor-version');
import { checkInt, checkTruthy } from '../lib/validation';
import { isVPNActive } from '../network';
import { doPurge, doRestart, serviceAction } from './common';
import { doPurge, doRestart, safeStateClone, serviceAction } from './common';
export function createV2Api(router: Router, applications: ApplicationManager) {
const { _lockingIfNecessary, deviceState } = applications;
@ -260,45 +260,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) {
router.get('/v2/local/target-state', async (_req, res) => {
const targetState = await deviceState.getTarget();
// We avoid using cloneDeep here, as the class
// instances can cause a maximum call stack exceeded
// error
// TODO: This should really return the config as it
// is returned from the api, but currently that's not
// the easiest thing due to the way they are stored and
// retrieved from the db - when all of the application
// manager is strongly typed, revisit this. The best
// thing to do would be to represent the input with
// io-ts and make sure the below conforms to it
const target: any = {
local: {
config: {},
},
dependent: {
config: {},
},
};
if (targetState.local != null) {
target.local = {
name: targetState.local.name,
config: _.cloneDeep(targetState.local.config),
apps: _.mapValues(targetState.local.apps, (app) => ({
appId: app.appId,
name: app.name,
commit: app.commit,
releaseId: app.releaseId,
services: _.map(app.services, (s) => s.toComposeObject()),
volumes: _.mapValues(app.volumes, (v) => v.toComposeObject()),
networks: _.mapValues(app.networks, (n) => n.toComposeObject()),
})),
};
}
if (targetState.dependent != null) {
target.dependent = _.cloneDeep(target.dependent);
}
const target = safeStateClone(targetState);
res.status(200).json({
status: 'success',