HTTP callout is sent with no error handling
ID |
apex.correctness.callout_no_error_handling |
Severity |
high |
Remediation Complexity |
medium |
Remediation Risk |
low |
Remediation Effort |
medium |
Resource |
Error Handling |
Language |
Apex |
Description
Reports an Http.send call that is not inside a try block and not inside a loop. It reports the absence of handling around the call, not the absence of a retry: a send inside a loop is left alone, because a loop is where a retry would be written.
Rationale
Http.send throws System.CalloutException for everything that goes wrong outside the remote application - a DNS failure, an expired certificate, a remote site setting that was never added in this org, the 120-second callout budget running out. Unhandled, that exception takes down the whole transaction, so a trigger-driven save fails in front of the user with an exception message about a callout, and a queued job ends up in the Apex Jobs list with no record of what it was working on. Remote systems are exactly the part of the transaction this org does not control, and a sandbox that has never had the endpoint registered fails this way on the first run.
The following code illustrates the pattern detected by this rule:
public class ErpGateway {
public HttpResponse fetchAccount(String externalId) {
HttpRequest req = new HttpRequest();
req.setEndpoint('callout:Erp/accounts/' + externalId);
req.setMethod('GET');
req.setTimeout(20000);
Http http = new Http();
// FLAGGED: HTTP callout is sent with no error handling
return http.send(req);
}
public String ping() {
HttpRequest req = new HttpRequest();
req.setEndpoint('callout:Erp/ping');
req.setTimeout(5000);
// FLAGGED: HTTP callout is sent with no error handling
HttpResponse res = new Http().send(req);
return res.getBody();
}
public HttpResponse fetchGuarded(HttpRequest req) {
Http http = new Http();
try {
return http.send(req);
} catch (System.CalloutException e) {
System.debug(LoggingLevel.ERROR, e.getMessage());
return null;
}
}
public HttpResponse fetchWithRetry(HttpRequest req) {
Http http = new Http();
for (Integer attempt = 0; attempt < 3; attempt++) {
HttpResponse res = http.send(req);
if (res.getStatusCode() == 200) {
return res;
}
}
return null;
}
public void notifyOwner(Messaging.SingleEmailMessage message) {
EmailDispatcher dispatcher = new EmailDispatcher();
dispatcher.send(message);
}
public HttpResponse fetchWithFallback(HttpRequest req) {
Http http = new Http();
try {
return http.send(req);
} catch (System.CalloutException e) {
return null;
} catch (Exception e) {
return null;
} finally {
System.debug('callout attempted');
}
}
public HttpResponse fetchUnhandled(HttpRequest req) {
Http http = new Http();
try {
// FLAGGED: HTTP callout is sent with no error handling
return http.send(req);
} finally {
System.debug('callout attempted');
}
}
}Remediation
Wrap the call in a try that catches System.CalloutException, and decide there what the transaction should do: record the failure against the record it belongs to, or re-throw a message that names the system that was unreachable. Where the work can be repeated safely, send from a Queueable that implements Database.AllowsCallouts and re-enqueue on failure, so the caller’s transaction is not held open for the remote system.