Extension to GlideRecord: addQuery2, Throws an exception if you pass a bad column name or bad operator
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
10-28-2015 01:31 PM
Have you ever been burned by the fact that GlideRecord's addQuery function silently tolerates bad column names? I have. This query was nearly "career-altering" for me, when, in a sprint review meeting with senior leadership after 8 months of development, something "unexpected" happened.
try {
var gr = new GlideRecord('change_task');
gr.addQuery('parent_change', current.sys_id);
gr.addQuery('short_description', 'CONTAINS', 'Build change');
gr.query();
while (gr.next()) {
gr.state = 3;
gr.update();
}
} catch (e) {
gs.logError('Error closing build change task for ' + current.number + ':' + e, 'Normal Change Workflow');
}
Can you spot the error? 'parent_change' is not a valid column name on the change_task table; it should be 'change_request'. But the code is wrapped in a try/catch, so it will throw if you make a mistake like this, right? Wrong! It just ignores line 3 due to the bad column name. The result is that ALL change tasks with "Build change" in the short description will be closed, not just the one that this workflow is running against.
In this particular situation, as the senior leaders were reviewing what we had built over a WebEx, a developer was following along on a different change. When they clicked their 'Close Build Change Task' UI Action form button on their change, it also unexpectedly closed the "Build change" task on the change that leadership was stepping through.
The plug got pulled on the release and we were in hot water. We found the bug, but the damage was done. We had carefully done our testing. What happened? Since the button technically worked (closed the task upon request), this code had made it through extensive testing by a very competent QA department.
This fiasco led me create a replacement for addQuery that will throw an exception if the query is malformed:
GlideRecord.prototype.addQuery2 = function () {
//@param colListWithDots is a string containing column names separated by dots.
//@param table is the name of the table
function checkDotWalking(colListWithDots, table) {
//@param table is a table name
//@param column is expected to be the name of a column on the table param
//@param referenceTypeColumn boolean that signals that this column is a reference-type col
// Returns an object with properties, success(bool), error (string), nextTable(string)
// nextTable property is only populated if the current column is a reference-type column
function checkCol(table, column, referenceTypeColumn) {
try {
var grCheckTable = GlideRecord(table);
grCheckTable.setLimit(1);
grCheckTable.query();
//Check for bad column name
if (!grCheckTable.isValidField(column)) {
return {
success: false,
error: column + " is not a valid field name for table: " + table
};
}
//Check for non reference-type column used for dot walking
if (referenceTypeColumn) {
if (grCheckTable[column].sys_meta.type_description != 'reference') {
return {
success: false,
error: 'Attempted dot walking on a column that is not a reference-type column:' + column
};
} else { //return succesful setting the nextTable, signal for continued dot walking
return {
success: true,
nextTable: grCheckTable[column].sys_meta.reference
}
}
}
// We have already passed every possible error condition. (Not a reference-type field. Reached the end of the dot walking)
return {
success: true
}
} catch (e3) {
throw 'Error in checkCol():' + e3;
}
}//end checkCol
//@param queryColString is the value of the first param from a call to addQuery (Column name)
//Returns an array filled with the tokens between non-alpha-chars (here 'dots')
//Example: if passed 'aaa.bbb.ccc.ddd', would return ['aaa','bbb','ccc','ddd']
function loadTokens(queryColString) {
try {
var matches = queryColString.match(/(\w+)/ig);
var tokens = [];
if (matches) {
for (var i = 0; i < matches.length; i++) {
tokens.push(matches[i]);
}
}
return tokens;
} catch (e4) {
throw 'Error in loadTokens():' + e4;
}
}
try {
var tokens = loadTokens(colListWithDots);
if (tokens.length > 1) {//We only want to do this if there is more than one token
for (var i = 0; i < tokens.length - 1; i++) {//Stop iterating one before the end. The last value is not a reference field
var columnName = tokens[i];
//Check the value to ensure that it is a valid col on that table (for a reference-type column)
var result = checkCol(table, columnName, true);
if (!result.success) {
return result;
}
table = result.nextTable;
}
var finalColumnName = tokens[tokens.length - 1];
//Check the last value to ensure that it is a valid col on that table (but not that it is a reference-type column)
result = checkCol(table, finalColumnName, false);
if (!result.success) {
return result;
}
return {
success: true
};
}
} catch (e2) {
throw 'Error in body of checkDotWalking():' + e2;
}
}
try {
//Cast params to strings
var a = arguments[0] + '';
var b = arguments[1] + '';
var c = arguments[2] + '';
//The first arg (column name) uses dot walking
if (/\w\.\w/.test(a)) {
var result = checkDotWalking(a, this.sys_meta.name);
if (!result.success) {
throw result.error;
}
} else if (!this.isValidField(a)) { //Throw an exception if the simple field name is bad (non-dot walking)
throw a + " is not a valid field name for table: " + this.sys_meta.name;
}
//3 parameter version
if (arguments[2]) {
var validOperators = ">,>=,=,!=,<,<=,STARTSWITH,ENDSWITH,IN,CONTAINS,DOES NOT CONTAIN";
//Throw an exception if the operator is bad
if (validOperators.indexOf(b) == -1) {
throw b + " is not a valid operation. Valid operations:" + validOperators;
}
this.addQuery(a, b, c);
return;
}
//2 parameter version
this.addQuery(a, b);
return;
} catch (e1) {
throw "Error:" + e1 + ". Source:GlideRecord.prototype.addQuery2";
}
};
You can now wrap your GlideRecord queries in a try catch and respond to bad field names.
It also throws when you foul up the operator.
Rewriting the initial "horror story" query from the beginning of the post to use addQuery2:
try {
var gr = new GlideRecord('change_task');
gr.addQuery2('parent_change', current.sys_id);//'parent_change' should be 'change_request'
gr.addQuery2('short_description', 'CONTAINS', 'Build change');
gr.query();
while (gr.next()) {
gr.state = 3;
gr.update();
}
} catch (e) {
gs.logError('Error closing build change task for ' + current.number + ':' + e, 'Normal Change Workflow');
}
Now the query will produce this error:
Normal Change Workflow: Error closing build change task for CHG0003345:Error:parent_change is not a valid field name for table: change_task. Source:GlideRecord.prototype.addQuery2: no thrown error
Much better!
Updated to reduce code complexity in checking validOperators based on feedback from sabell2012
Message was edited by: Updated to throw an error based on bad dot walking.
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
10-30-2015 07:59 AM
I have updated the code to change the loop control variable in the for loop. I found that the use of the varible "i" was causing a serious performance issue for some queries. This variable must be conflicting with an outside "i" variable. To avoid this I have changed the LCV's name to "ijk". Hopefully that's an uncommon choice.

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
10-30-2015 08:29 AM
Trey:
I have a suggestion to reduce the code complexity. How about this?
Replace:
var validOperators = ['>', '>=', '=', '!=', '<', '<=', 'STARTSWITH', 'ENDSWITH', 'IN', 'CONTAINS', 'DOES NOT CONTAIN'];
var validOp = false;
for (var ijk = 0; ijk < validOperators.length; ijk++) {
if (b == validOperators[ijk]) {
validOp = true;
break;
}
}
//Throw an exception if the operator is bad
if (!validOp) {
throw b + " is not a valid operation. Valid operations:" + validOperators.toString();
}
With:
var validOperators = ['>', '>=', '=', '!=', '<', '<=', 'STARTSWITH', 'ENDSWITH', 'IN', 'CONTAINS', 'DOES NOT CONTAIN'];
//Throw an exception if the operator is bad
if (validOperators.indexOf(b) == -1) {
throw b + " is not a valid operation. Valid operations:" + validOperators.toString();
}
Steven.

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
10-30-2015 09:13 AM
Trey:
Ooops. I have been doing so much with prototypes recently that I forgot to do the array.indexOf include! That feature is not a part of the OOtB JavaScript language and needs to be added in. This article is where I show how to do that.
Mini-Lab: Adding a Select <<field>> to the Extended GlideRecord
Steven.

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
10-30-2015 10:00 AM
Great discussion guys... Bookmarked !!!!
This deserves more than 5 points!

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
05-12-2016 10:15 AM
Trey,
I'm new to ServiceNow and I was wondering where you added this into ServiceNow so you can call .addQuery2() from any script (JavaScript)?
Thanks,
James