Database operation results are never inspected
ID |
apex.correctness.saveresult_errors_unchecked |
Severity |
high |
Remediation Complexity |
medium |
Remediation Risk |
low |
Remediation Effort |
medium |
Resource |
Error Handling |
Language |
Apex |
Description
Reports a method that assigns the results of a Database.insert / update / upsert / delete call and never iterates them.
Rationale
The Database.* methods exist to allow partial success: rows that fail are reported in the result list instead of throwing, and the transaction carries on. That is only useful if something reads the list. Left unread, every rejected row - a validation rule, a required field, a duplicate key - disappears without an exception, without a log line and without a rollback, and the caller reports success for a load that only partly happened. The missing records are found later by whoever reconciles the two systems, with nothing in the org to say what went wrong or when.
The following code illustrates the pattern detected by this rule:
public class BulkLoader {
// FLAGGED: Database operation results are never inspected
public void loadAccounts(List<Account> accounts) {
List<Database.SaveResult> results = Database.insert(accounts, false);
System.debug('loaded ' + accounts.size());
}
// FLAGGED: Database operation results are never inspected
public void upsertContacts(List<Contact> contacts) {
List<Database.UpsertResult> results = Database.upsert(contacts, Contact.Fields.Email, false);
System.debug('upserted');
}
public void loadAccountsChecked(List<Account> accounts) {
List<Database.SaveResult> results = Database.insert(accounts, false);
for (Database.SaveResult r : results) {
if (!r.isSuccess()) {
System.debug(LoggingLevel.ERROR, r.getErrors());
}
}
}
public void loadAccountsAllOrNone(List<Account> accounts) {
insert accounts;
}
}Remediation
Iterate the results, and for each one that is not successful, read getErrors() and do something with it - write a row to a logging object, collect the failures for the caller, or add the error back onto the record. Where partial success is not wanted at all, use the DML statement (insert accounts;) so a failure throws and rolls the transaction back.