Skip to content

Commit

Permalink
findValueRef and findSubValueRef return undefined for nonexistent path
Browse files Browse the repository at this point in the history 
Previously findValueRef and findSubValueRef threw an Error for nonexistant paths. In general throwing an error from the Vireo API functions means that Vireo is in an unrecoverable state.

However, there were tests that would cause errors to be thrown for nonexistent paths but continue to use the Vireo instance for additional tests. This now resulted in failures in the asm.js buid after a compiler upgrade.

Instead of modifying the tests to use new Vireo instances between runs, the behavior of findValueRef and findSubValueRef was modified to safely return undefined if a path is not found. This has been a desired behavior on the ASW side as well.
  • Loading branch information
@rajsite
rajsite committed Sep 27, 2018
1 parent 1b950b2 commit aca2198
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 78 deletions.
22 changes: 14 additions & 8 deletions source/io/module_eggShell.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,16 +232,19 @@ var assignEggShell;
var dataStackPointer = Module.stackAlloc(POINTER_SIZE);

var eggShellResult = Module._EggShell_FindValue(Module.eggShell.v_userShell, viStackPointer, pathStackPointer, typeStackPointer, dataStackPointer);
if (eggShellResult !== EGGSHELL_RESULT.SUCCESS) {
if (eggShellResult !== EGGSHELL_RESULT.SUCCESS && eggShellResult !== EGGSHELL_RESULT.OBJECT_NOT_FOUND_AT_PATH) {
throw new Error('A ValueRef could not be made for the following reason: ' + eggShellResultEnum[eggShellResult] +
' (error code: ' + eggShellResult + ')' +
' (vi name: ' + vi + ')' +
' (path: ' + path + ')');
}

var typeRef = Module.getValue(typeStackPointer, 'i32');
var dataRef = Module.getValue(dataStackPointer, 'i32');
var valueRef = Module.eggShell.createValueRef(typeRef, dataRef);
var typeRef, dataRef, valueRef;
if (eggShellResult !== EGGSHELL_RESULT.OBJECT_NOT_FOUND_AT_PATH) {
typeRef = Module.getValue(typeStackPointer, 'i32');
dataRef = Module.getValue(dataStackPointer, 'i32');
valueRef = Module.eggShell.createValueRef(typeRef, dataRef);
}

Module.stackRestore(stack);
return valueRef;
Expand All @@ -255,16 +258,19 @@ var assignEggShell;
var dataStackPointer = Module.stackAlloc(POINTER_SIZE);

var eggShellResult = Module._EggShell_FindSubValue(Module.eggShell.v_userShell, valueRef.typeRef, valueRef.dataRef, subPathStackPointer, typeStackPointer, dataStackPointer);
if (eggShellResult !== EGGSHELL_RESULT.SUCCESS) {
if (eggShellResult !== EGGSHELL_RESULT.SUCCESS && eggShellResult !== EGGSHELL_RESULT.OBJECT_NOT_FOUND_AT_PATH) {
throw new Error('A ValueRef could not be made for the following reason: ' + eggShellResultEnum[eggShellResult] +
' (error code: ' + eggShellResult + ')' +
' (type name: ' + Module.typeHelpers.typeName(valueRef.typeRef) + ')' +
' (subpath: ' + subPath + ')');
}

var typeRef = Module.getValue(typeStackPointer, 'i32');
var dataRef = Module.getValue(dataStackPointer, 'i32');
var subValueRef = Module.eggShell.createValueRef(typeRef, dataRef);
var typeRef, dataRef, subValueRef;
if (eggShellResult !== EGGSHELL_RESULT.OBJECT_NOT_FOUND_AT_PATH) {
typeRef = Module.getValue(typeStackPointer, 'i32');
dataRef = Module.getValue(dataStackPointer, 'i32');
subValueRef = Module.eggShell.createValueRef(typeRef, dataRef);
}

Module.stackRestore(stack);
return subValueRef;
Expand Down
6 changes: 2 additions & 4 deletions test-it/karma/publicapi/AllocateTypes.Test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,8 @@ describe('Vireo public API allows', function () {
};

describe('error handling', function () {
// Use a memory aligned invalid addess to avoid alignment warnings in debug builds
// Debug builds enable SAFE_HEAP which warns on unaligned access: https://kripken.github.io/emscripten-site/docs/porting/guidelines/portability_guidelines.html#other-issues
// Wasm supports unaligned memory access, although it may cause performance degradation
var invalidAddress = 1024;
// Use null pointer for an invalid address. Random values can have unexpected results.
var invalidAddress = 0;
describe('for allocateData', function () {
it('throws an InvalidTypeRef error given an invalid typeRef', function () {
var invalidTypeRef = invalidAddress;
Expand Down
72 changes: 24 additions & 48 deletions test-it/karma/publicapi/FindSubValueRef.Test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,9 @@ describe('The Vireo EggShell findSubValueRef', function () {
expect(invalidValueRef).toThrow();
});

it('throws for a nonexistant path', function () {
var invalidPath = function () {
vireo.eggShell.findSubValueRef(valueRef, 'nonexistantpath');
};

expect(invalidPath).toThrow();
it('return undefined for a nonexistant path', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, 'nonexistantpath');
expect(nonExistantValueRef).toBeUndefined();
});

it('finds values of cluster elements', function () {
Expand All @@ -87,20 +84,14 @@ describe('The Vireo EggShell findSubValueRef', function () {
valueRef = vireo.eggShell.findValueRef(viName, arrayOfBooleansPath);
});

it('throws for invalid index', function () {
var invalidIndex = function () {
vireo.eggShell.findSubValueRef(valueRef, '-1');
};

expect(invalidIndex).toThrow();
it('return undefined for invalid index', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, '-1');
expect(nonExistantValueRef).toBeUndefined();
});

it('throws for index out of bounds', function () {
var invalidIndex = function () {
vireo.eggShell.findSubValueRef(valueRef, '10');
};

expect(invalidIndex).toThrow();
it('return undefined for index out of bounds', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, '10');
expect(nonExistantValueRef).toBeUndefined();
});

it('finds a value', function () {
Expand All @@ -116,20 +107,14 @@ describe('The Vireo EggShell findSubValueRef', function () {
valueRef = vireo.eggShell.findValueRef(viName, ndimArrayPath);
});

it('throws for an invalid path format', function () {
var invalidPathFormat = function () {
vireo.eggShell.findSubValueRef(valueRef, '0,0.0');
};

expect(invalidPathFormat).toThrow();
it('return undefined for an invalid path format', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, '0,0.0');
expect(nonExistantValueRef).toBeUndefined();
});

it('throws for index out of bounds', function () {
var indexOutOfBounds = function () {
vireo.eggShell.findSubValueRef(valueRef, '2,0,0');
};

expect(indexOutOfBounds).toThrow();
it('return undefined for index out of bounds', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, '2,0,0');
expect(nonExistantValueRef).toBeUndefined();
});

it('finds values for comma-separated indexes', function () {
Expand All @@ -150,12 +135,9 @@ describe('The Vireo EggShell findSubValueRef', function () {
valueRef = vireo.eggShell.findValueRef(viName, arrayOfClustersPath);
});

it('throws for invalid path format', function () {
var invalidPathFormat = function () {
vireo.eggShell.findSubValueRef(valueRef, '0,bool');
};

expect(invalidPathFormat).toThrow();
it('return undefined for invalid path format', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, '0,bool');
expect(nonExistantValueRef).toBeUndefined();
});

it('finds values for indexes followed by "." field name', function () {
Expand All @@ -177,20 +159,14 @@ describe('The Vireo EggShell findSubValueRef', function () {
valueRef = vireo.eggShell.findValueRef(viName, clusterOfArraysPath);
});

it('throws for invalid path format', function () {
var invalidPathFormat = function () {
vireo.eggShell.findSubValueRef(valueRef, 'booleans,0');
};

expect(invalidPathFormat).toThrow();
it('return undefined for invalid path format', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, 'booleans,0');
expect(nonExistantValueRef).toBeUndefined();
});

it('throws for index out of bounds', function () {
var indexOutOfBounds = function () {
vireo.eggShell.findSubValueRef(valueRef, 'booleans.3');
};

expect(indexOutOfBounds).toThrow();
it('return undefined for index out of bounds', function () {
var nonExistantValueRef = vireo.eggShell.findSubValueRef(valueRef, 'booleans.3');
expect(nonExistantValueRef).toBeUndefined();
});

it('finds values for fields followed by "." index', function () {
Expand Down
24 changes: 9 additions & 15 deletions test-it/karma/publicapi/FindValueRef.Test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,19 @@ describe('The Vireo EggShell findValueRef api can', function () {
expect(valueRef.dataRef).not.toBe(0);
});

it('to throw for a nonexistant vi name', function () {
var invalidViName = function () {
vireo.eggShell.findValueRef('nonexistantvi', pathName);
};
expect(invalidViName).toThrowError(/ObjectNotFoundAtPath/);
it('to return undefined for a nonexistant vi name', function () {
var valueRef = vireo.eggShell.findValueRef('nonexistantvi', pathName);
expect(valueRef).toBeUndefined();
});

it('to throw for an empty vi name', function () {
var invalidViName = function () {
vireo.eggShell.findValueRef('', pathName);
};
expect(invalidViName).toThrowError(/ObjectNotFoundAtPath/);
it('to return undefined for an empty vi name', function () {
var valueRef = vireo.eggShell.findValueRef('', pathName);
expect(valueRef).toBeUndefined();
});

it('to throw for a nonexistant path', function () {
var invalidPath = function () {
vireo.eggShell.findValueRef(viName, 'nonexistantvalue');
};
expect(invalidPath).toThrowError(/ObjectNotFoundAtPath/);
it('to return undefined for a nonexistant path', function () {
var valueRef = vireo.eggShell.findValueRef(viName, 'nonexistantvalue');
expect(valueRef).toBeUndefined();
});

it('to return a typeRef for the the local scope of a VI for an empty path', function () {
Expand Down
4 changes: 1 addition & 3 deletions test-it/karma/publicapi/TypedArray.Test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ describe('The Vireo EggShell Typed Array api', function () {
readTypedArray('arrayString');
}).toThrowError(/UnexpectedObjectType/);

expect(function () {
readTypedArray('nonExistantPath');
}).toThrowError(/ObjectNotFoundAtPath/);
expect(vireo.eggShell.findValueRef(viName, 'nonExistantPath')).toBeUndefined();

expect(function () {
readTypedArray('scalarUInt32');
Expand Down

0 comments on commit aca2198

Please sign in to comment.