I am implementing the PayPal Instant Payment Notifcation (IPN) in my .NET Core Web API and the reference implementation from PayPal uses a Task.Run() from within a Web API controller.
Visual Studio gives me a green wavy underline for that call with the hint, that this call is not waited for. Which is intended in this special case, because that’s what the PayPal workflow demands.
- Incoming IPN Message
- Start a Task to Verify the IPN Message
- Return OK for 1.
[HttpPost]
public async Task<IActionResult> Receive()
{
IPNContext context = new IPNContext()
{
IPNRequest = Request
};
using (var ms = new StreamReader(context.IPNRequest.Body))
{
context.RequestBody = await ms.ReadToEndAsync();
}
//Fire and forget verification task
Task.Run(() => VerifyTask(context)); // Send back the same IPN-message with added "_notify-validate"-parameter and then continue to process from there
//Reply back a 200 code
return Ok();
}
private void VerifyTask(IPNContext ipnContext)
{
try
{
var verificationRequest = WebRequest.Create("https://www.sandbox.paypal.com/cgi-bin/webscr");
//Set values for the verification request
verificationRequest.Method = "POST";
verificationRequest.ContentType = "application/x-www-form-urlencoded";
//Add cmd=_notify-validate to the payload
string strRequest = "cmd=_notify-validate&" + ipnContext.RequestBody;
verificationRequest.ContentLength = strRequest.Length;
//Attach payload to the verification request
using (StreamWriter writer = new StreamWriter(verificationRequest.GetRequestStream(), Encoding.ASCII))
{
writer.Write(strRequest);
}
//Send the request to PayPal and get the response
using (StreamReader reader = new StreamReader(verificationRequest.GetResponse().GetResponseStream()))
{
ipnContext.Verification = reader.ReadToEnd();
}
}
catch (Exception exception)
{
_logger.LogDebug($"IPN-REQUEST::VerifyTask::ERROR => {exception.Message}");
}
ProcessVerificationResponse(ipnContext); // That is my internal processing of the Event
}
So my questions are:
- Is this good practice in general?
- How can I avoid the wavy underline hint?
Best regards
2
Answers
No it’s not a good practice because not-awaited tasks are (like you have written) fire and forget without a correct context around to handle exceptions or failures when calling the method. So in your case the consumer simply receives an
OK
(200) so from his perspective everything is ok. It really depends on your workflow (Especially whatIPNContext
is doing). I personally would expect anAccepted
(202) if you do a fire & forget and give the consumer another Api to query for the status I think then it is ok.You can of course trick on Visual Studio by simply writing:
_ = Task.Run(...)
Better solution is just
await
the task like Visual Studio is recommanding you. I don’t know the background why theawait
is missing in the PayPal examples but maybe just ask them.Imo a not-awaited
Task
always looks like a code smell because a correct exception/failure handling is then not possible from the method caller.First, I’d say that all API code samples should be taken with a grain of salt. Even for large companies, there’s usually only a small team (or single person) who has to maintain the code samples for a half dozen (or more) languages. It’s simply not possible in that situation to have high-quality API code samples (or client libraries, for that matter).
In this case, there are several problems with the sample code:
WebRequest
actually being in the code.Task.Run
on ASP.NET.If the PayPal workflow can handle it, you could remove the
Task.Run
completely.If the PayPal workflow requires offline processing, then the proper solution is to use a distributed architecture. I.e., save the request parameters in a reliable queue (Azure Storage Queue / Amazon SQS / local on-disk queue) and have an independent processor (Azure Function / Amazon Lambda / ASP.NET Core Worker) that reads from the queue and sends the request to the PayPal API. Hopefully using something more modern than
WebRequest
.What I call the "proper solution" is quite a bit more complex than would be appropriate for a code sample. Which is another reason why code samples are often more about "here’s how you can call this API", not about "here’s how your code should look".