JeremySkinner / SagePayMvc

ASP.NET MVC Integration for SagePay
Apache License 2.0
31 stars 31 forks source link

Notification URL is incorrectly parsed #16

Closed rippo closed 9 years ago

rippo commented 9 years ago

I have a strange issue where the Notification URL is being incorrectly parsed...

2015-09-25_16-13-37

My web.config is:-

  <sagePay>
    <add key="NotificationHostName" value="http://dev.wildesoft.net:80/" />
    <add key="VatMultiplier" value="1.20" />
    <add key="VendorName" value="1234567890" />
    <add key="Mode" value="Test" />  <!--Test or Live-->
  </sagePay>

Can I override this somewhere to get past this issue for now?

JeremySkinner commented 9 years ago

NotificationHostName should be just that - a host name. It shouldn't include protocols or ports. The full URL is constructed using MVC's UrlHelper, which will add in the protocol (based on either a configuration value, or it'll default to http), and the port will always use the same port you're running the website under. So in the web config you should just have this:

<add key="NotificationHostName" value="dev.wildesoft.net" />

...and optionally you can specify a protocol (if you don't it'll default to http)

<add key="Protocol" value="https" />
rippo commented 9 years ago

ah ta, I see misread that somewhere. Yes this is now correct. However I now always get this error on sage pay..... (which is what lead me to my merry dance of playing with the url in the first place)

Server error 5006: Unable to redirect to Vendor's web site. The Vendor failed to provide a RedirectionURL.

HTTP error 500: The request was unsuccessful due to an unexpected condition encountered by the server.

My code does not get hit as I have breakpoints on my paymentresponse controller

JeremySkinner commented 9 years ago

That's probably because of the port...looking at your screenshot, it's running on port 37184. Unless you've got this port open publicly, then sagepay won't be able to send you a response.

rippo commented 9 years ago

yes the port is open publicly and can be got to from a browser. e.g. http://dev.wildesoft.net:37184/ albeit slowly

JeremySkinner commented 9 years ago

OK, how are you sending back your RedirectUrl from your notification action? Are you using SagePayMvc's ActionResults?

Basically, the Notification action needs to send a response back to SagePay with a RedirectUrl telling it where to send the user for a successful or failed payment, and that error usually means that you didn't give SagePay the redirectionURL, or it was in the wrong format.

The ActionResults in SagePayMvc should take care of this for you, take a look at the example controller: https://github.com/JeremySkinner/SagePayMvc/blob/master/Sample/SagePayMvc.Sample/Controllers/PaymentResponseController.cs

public class PaymentResponseController : Controller {

  public ActionResult Notify(SagePayResponse response) {
    // SagePay should have sent back the order ID
    if (string.IsNullOrEmpty(response.VendorTxCode)) {
      return new ErrorResult();
    }

    // Get the order out of our "database"
    var order = _orderRepository.GetById(response.VendorTxCode);

    // IF there was no matching order, send a TransactionNotfound error
    if (order == null) {
      return new TransactionNotFoundResult(response.VendorTxCode);
    }

    // All good - tell SagePay it's safe to charge the customer.
    // This will include a RedirectUrl based on what's in the web.config
    return new ValidOrderResult(order.VendorTxCode, response);
  }

  public ActionResult Failed(string vendorTxCode) {
    return View();
  }

  public ActionResult Success(string vendorTxCode) {
    return View();
  }
}

...and here's the corresponding config entries that specify which actions to use (SuccessAction and FailedAction): https://github.com/JeremySkinner/SagePayMvc/blob/master/Sample/SagePayMvc.Sample/Web.config

<sagePay>
  <!-- The public-facing hostname that SagePay can use to contact the site -->
  <add key="NotificationHostName" value="my.external.hostname" />
  <!-- The protocol defaults to http, but you can override that to https with the following setting -->
  <!-- <add key="Protocol" value="https" /> -->
  <!-- Your notification controller -->
  <add key="NotificationController" value="PaymentResponse" />
  <!-- Your notification action. These three settings together are used to build the notification URL -->
  <!-- EG: http://my.external.hostname/PaymentResponse/Notify -->
  <add key="NotificationAction" value="Notify" />
  <!-- Action names for URLS that the user will be directed to after payment either succeeds or fails -->
  <!-- The URL is constructed from the notificationHostName and NotificationController. -->
  <!-- Eg: http://my.external.hostname/PaymentResponse/Success -->
  <add key="SuccessAction" value="Success" />
  <add key="FailedAction" value="Failed" />

  <!-- VAT multiplier. Currently at 20% -->
  <add key="VatMultiplier" value="1.2" />
  <!-- Name of vendor. You will need to change this -->
  <add key="VendorName" value="MyVendorName" />
  <!-- Simulator, Test or Live -->
  <add key="Mode" value="Simulator" />
</sagePay>
rippo commented 9 years ago

ah it might be it, got this

return Redirect(response.NextURL);

instead of this

return new ValidOrderResult(order.VendorTxCode, response); (or equiv)

JeremySkinner commented 9 years ago

Ah yes that'll be the problem - you can't redirect at that stage, basically your Notification action just needs to write out some text to the response (the ValidOrderResult, ErrorResult do this for you - see here)

rippo commented 9 years ago

Ah no sorry that's not my issue, your code is the notification that is applied after I click the "pay now" button on sage pay. Sage pay spins for a while showing

"Your payment is being authorised, it won't take long"

whilst this is happening my code is not hit at all (I have it running with breakpoints)

2015-09-25_16-54-36

JeremySkinner commented 9 years ago

What do you have in your web.config for NotificationAction, SuccessAction and FailedAction (or are you using the defaults)?

rippo commented 9 years ago

I don't looking for that now on your repo

JeremySkinner commented 9 years ago

Ok - that's fine - you don't need them (they just default to "Index", "Success" and "Failed" if you don't specify) - just wanted to check that the notification action matches.

So it looks like for some reason SagePay's response isn't hitting the right URL. Have you tried sending a post to the notification action from outside your network? (Just to make sure there's definitely no firewall issues?)

rippo commented 9 years ago

that's what I am going to try next, also going to put some logging to see whether my code is hit from sage pay. Although if it was hitting my code I can;t see why it is not hitting the breakpoint.

JeremySkinner commented 9 years ago

Yeah it definitely sounds like it's not being hit at all...this is always the most difficult bit to troubleshoot with SagePay integrations in my experience!

It might also be worth trying it on port 80 just to make sure that's not the issue (I don't think it should be, but would be worth ruling out)

rippo commented 9 years ago

I did try that, but will do again, ta

JeremySkinner commented 9 years ago

Another thing to try might be to turn on IIS's logging, to see whether there's a request coming in at all and just going to the wrong url.

rippo commented 9 years ago

yes I am using VS2015 and using a users applicationhost Will mess about with this to get port 80 as only port open

rippo commented 9 years ago

blinking eck thats it, looks like port 80 or 443 is needed for sage pay and not custom ports EVEN in Test mode! Sorry about that got there in end

JeremySkinner commented 9 years ago

Ah, that's annoying! Glad you figured it out though!

rippo commented 9 years ago

Thanks for all your help, will close this ticket