Skip to content

log row values when exception during script execution #2704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ankurjuneja merged 4 commits into from
Nov 4, 2021
Merged

log row values when exception during script execution #2704

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a9352a
log row values when exception during script execution
ankurjuneja Oct 20, 2021
1271ed6
Improve error messages, and avoid leaking PHI
Nov 2, 2021
2dbf0de
Merge branch 'release21.7-SNAPSHOT' into 21.7_fb_logNormalizingDate
Nov 2, 2021
a2cfa80
Merge branch 'release21.7-SNAPSHOT' into 21.7_fb_logNormalizingDate
Nov 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 37 additions & 15 deletions api/src/org/labkey/api/data/triggers/ScriptTrigger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.jetbrains.annotations.Nullable;
import org.labkey.api.data.Container;
import org.labkey.api.data.DbScope;
import org.labkey.api.data.PHI;
import org.labkey.api.data.TableInfo;
import org.labkey.api.query.BatchValidationException;
import org.labkey.api.query.ValidationException;
Expand All @@ -35,6 +36,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.function.Supplier;

/**
* Implements a trigger for table operations backed by JavaScript code.
Expand Down Expand Up @@ -67,16 +69,29 @@ public boolean canStream()
return false;
}

/**
* To avoid leaking PHI through log files, avoid including the full row info in the error detail when any of the
* columns in the target table is considered PHI
*/
private Supplier<String> filterErrorDetailByPhi(TableInfo table, Supplier<String> errorDetail)
{
if (table.getMaxPhiLevel() != PHI.NotPHI)
{
return () -> null;
}
return errorDetail;
}

@Override
public void init(TableInfo table, Container c, User user, TableInfo.TriggerType event, BatchValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "init", errors, extraContext, event.name().toLowerCase());
invokeTableScript(table, c, user, "init", errors, extraContext, () -> null, event.name().toLowerCase());
}

@Override
public void complete(TableInfo table, Container c, User user, TableInfo.TriggerType event, BatchValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "complete", errors, extraContext, event.name().toLowerCase());
invokeTableScript(table, c, user, "complete", errors, extraContext, () -> null, event.name().toLowerCase());
}


Expand All @@ -85,75 +100,82 @@ public void beforeInsert(TableInfo table, Container c,
User user, @Nullable Map<String, Object> newRow,
ValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "beforeInsert", errors, extraContext, newRow);
invokeTableScript(table,
c,
user,
"beforeInsert",
errors,
extraContext,
filterErrorDetailByPhi(table, () -> "New row data: " + newRow),
newRow);
}

@Override
public void beforeUpdate(TableInfo table, Container c,
User user, @Nullable Map<String, Object> newRow, @Nullable Map<String, Object> oldRow,
ValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "beforeUpdate", errors, extraContext, newRow, oldRow);
invokeTableScript(table, c, user, "beforeUpdate", errors, extraContext, filterErrorDetailByPhi(table, () -> "New row: " + newRow + ". Old row: " + oldRow), newRow, oldRow);
}

@Override
public void beforeDelete(TableInfo table, Container c,
User user, @Nullable Map<String, Object> oldRow,
ValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "beforeDelete", errors, extraContext, oldRow);
invokeTableScript(table, c, user, "beforeDelete", errors, extraContext, filterErrorDetailByPhi(table, () -> "Old row: " + oldRow), oldRow);
}

@Override
public void afterInsert(TableInfo table, Container c,
User user, @Nullable Map<String, Object> newRow,
ValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "afterInsert", errors, extraContext, newRow);
invokeTableScript(table, c, user, "afterInsert", errors, extraContext, filterErrorDetailByPhi(table, () -> "New row: " + newRow), newRow);
}

@Override
public void afterUpdate(TableInfo table, Container c,
User user, @Nullable Map<String, Object> newRow, @Nullable Map<String, Object> oldRow,
ValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "afterUpdate", errors, extraContext, newRow, oldRow);
invokeTableScript(table, c, user, "afterUpdate", errors, extraContext, filterErrorDetailByPhi(table, () -> "New row: " + newRow + ". Old row: " + oldRow), newRow, oldRow);
}

@Override
public void afterDelete(TableInfo table, Container c,
User user, @Nullable Map<String, Object> oldRow,
ValidationException errors, Map<String, Object> extraContext)
{
invokeTableScript(table, c, user, "afterDelete", errors, extraContext, oldRow);
invokeTableScript(table, c, user, "afterDelete", errors, extraContext, filterErrorDetailByPhi(table, () -> "Old row: " + oldRow), oldRow);
}


protected void invokeTableScript(TableInfo table, Container c, User user, String methodName, BatchValidationException errors, Map<String, Object> extraContext, Object... args)
protected void invokeTableScript(TableInfo table, Container c, User user, String methodName, BatchValidationException errors, Map<String, Object> extraContext, Supplier<String> errorDetail, Object... args)
{
Object[] allArgs = Arrays.copyOf(args, args.length+1);
allArgs[allArgs.length-1] = errors;
Boolean success = _invokeTableScript(c, user, Boolean.class, methodName, extraContext, allArgs);
Boolean success = _invokeTableScript(c, user, Boolean.class, methodName, extraContext, errorDetail, allArgs);
if (success != null && !success)
errors.addRowError(new ValidationException(methodName + " validation failed"));
if (isConnectionClosed(table.getSchema().getScope()))
errors.addRowError(new ValidationException("script error: " + methodName + " trigger closed the connection, possibly due to constraint violation"));
}


protected void invokeTableScript(TableInfo table, Container c, User user, String methodName, ValidationException errors, Map<String, Object> extraContext, Object... args)
protected void invokeTableScript(TableInfo table, Container c, User user, String methodName, ValidationException errors, Map<String, Object> extraContext, Supplier<String> errorDetail, Object... args)
{
Object[] allArgs = Arrays.copyOf(args, args.length+1);
allArgs[allArgs.length-1] = errors;
Boolean success = _invokeTableScript(c, user, Boolean.class, methodName, extraContext, allArgs);
Boolean success = _invokeTableScript(c, user, Boolean.class, methodName, extraContext, errorDetail, allArgs);
if (success != null && !success)
errors.addGlobalError(methodName + " validation failed");
if (isConnectionClosed(table.getSchema().getScope()))
errors.addGlobalError("script error: " + methodName + " trigger closed the connection, possibly due to constraint violation");
}


private <T> T _invokeTableScript(Container c, User user, Class<T> resultType, String methodName, Map<String, Object> extraContext, Object... args)
private <T> T _invokeTableScript(Container c, User user, Class<T> resultType, String methodName, Map<String, Object> extraContext, Supplier<String> errorDetail, Object... args)
{
try
{
Expand All @@ -163,7 +185,6 @@ private <T> T _invokeTableScript(Container c, User user, Class<T> resultType, St
// Push a view context if we don't already have one available. It will be pulled if labkey.js
// is required by the trigger script being invoked, via the call to PageFlowUtil.jsInitObject() in
// server/modules/core/resources/scripts/labkey/init.js
//noinspection deprecation
viewContextResetter = ViewContext.pushMockViewContext(user, c, new ActionURL("dummy", "dummy", c));
}
try
Expand Down Expand Up @@ -195,7 +216,8 @@ private <T> T _invokeTableScript(Container c, User user, Class<T> resultType, St
}
catch (NoSuchMethodException | ScriptException e)
{
throw new UnexpectedException(e);
String extraErrorMessage = errorDetail.get();
throw UnexpectedException.wrap(e, "Script execution failed for " + methodName + "()" + (extraErrorMessage == null ? "" : " " + extraErrorMessage));
}

return null;
Expand Down