Skip to content
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.

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

Recurly_Invoice::getInvoicePdf doesn't validate response code #465

Closed
glaubinix opened this issue Mar 2, 2020 · 3 comments
Closed

Recurly_Invoice::getInvoicePdf doesn't validate response code #465

glaubinix opened this issue Mar 2, 2020 · 3 comments
Assignees
Labels
bug V2 V2 Client

Comments

@glaubinix
Copy link

Describe the bug

The method Recurly_Invoice::getInvoicePdf($invoiceNumber) doesn't validate the response code which means that if the server returns any status code >=400 then the method returns a HTML document instead of the pdf content.

This is not so much an issue for 404 or similar errors because we can validate that the invoice exists before downloading it but is more of an issues for 5XX errors.

To Reproduce

Recurly_Invoice::getInvoicePdf($nonExistingInvoiceNumber)

Expected behavior

The method throws an exception like other methods in the library if an unexpected response code was received.

Your Environment

  • Which version of this library are you using? 2.12.11
  • Which version of php are you using? 7.2
@glaubinix glaubinix added the bug? label Mar 2, 2020
@douglasmiller douglasmiller added bug V2 V2 Client and removed bug? labels Mar 3, 2020
@douglasmiller
Copy link
Contributor

@glaubinix, thank you for reporting this issue. We will take a look at implementing a more robust error handling mechanism for this functionality.

@douglasmiller
Copy link
Contributor

@glaubinix Thank you again for reporting this issue. We have updated the error handling for the pdf requests and an updated client will be published soon.

I'd also like to point out that we just launched a new version of the PHP client library that leverages the v3 API. The current 3.0.0 version of the library requires at least PHP 7.3, but I've dropped that requirement down to 7.2 in #473. Take a look and see if that is something that you'd be interested in using.

@glaubinix
Copy link
Author

@douglasmiller thank you! Will certainly have a look at the v3 API client as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V2 V2 Client
Projects
None yet
Development

No branches or pull requests

2 participants