There is an overwhelming amount of good advice in this post.

"Don't fail silently."

That can't be stressed enough, especially when dealing with critical data (in this case, financial).

"Like TDD, it's crucial to perform this error-handling analysis early because doing so will likely change your module's interface."

Truer advice has never been given. To me, writing my test platform is the most critical facet to interface design. It dictates it. I'm literally writing tests (nothing more than scripts that are true users of my software) that guides me to the end interface appearance. For me, my error messages (after so many years of coding) are generally logical, but through writing tests to cover edge cases, error messages become more refined, appropriate and understandable.

"with the module user expected to check this error property after calling each method."

Few will do that. You'll receive emails and bug tickets. Then you'll have to hand-hold to get someone to repro the issue which will never be the same again, and the bug goes unfound.

IMHO, a warning is something that you can let your users check for themselves. True errors however should be forefront. A user of the interface should have to do work to work around the error, not hear from someone that something isn't working causing all manner of teams to troubleshoot, then search out the error. Show them up front... somethings broken severely. It might be you, it might be me, but we can't continue until action is taken.

It is counter-productive to force users to go seek out whether there's an error under the hood or not. Break the caller, and allow them to decide whether they want to put an eval{} around it or not.

Failing silently is a nightmare and a frustration across the board.

Magnificent post, eyepopslikeamosquito!

-stevieb

Throw an exception when ... an error is so rare that the programmer is likely to forget to check for it

Yes. Furthermore, I would expand this to include "or you believe that the underlying triggering condition should never occur". I'm sure we have all written code with labyrinthine logical paths and found one branch which should never be reached. In such a scenario I throw an exception in that branch to say as much and briefly indicate why. Such exceptions end up being thrown on a bafflingly frequent basis and this otherwise-unhandled unexpected condition is a really good candidate for an abrupt termination. What else should you do when the logic fails so utterly?

Failing silently causes the following problems: Users wonder whether something has gone wrong. ("Why did my order not go through?")

Much worse than "why didn't it go through?" is "has it gone through or not?" - I've encountered this a few times recently when ordering online and it is frankly infuriating. You end up wrestling not only with the vendor's poor interface but also your card provider's only slightly better one trying to work out if you have been charged zero, one or many times and how many widgets, if any, you can expect to turn up in 5 to 7 working days. Such vendors are migrated swiftly to the bargepole list.

> I would expand this to include "or you believe that the underlying triggering condition should never occur" ... What else should you do when the logic fails so utterly?

Yes. You just reminded me of my first boss, great salesman, engaging personality ... still my favourite boss. Though not a great programmer, he'd hacked out a system in record time and managed to sell it to a chain of bakeries. His system made us a lot of money and was working remarkably well until, in the middle of the night, we get a support call from a very sheepish bakery worker, reluctant to explain exactly what the problem was. Finally, she said the system had stopped with "What the f*!!? am I doing here?" displayed on the screen. :)

I'd forgotten to check Perl Best Practices when writing the root node; from its Error Handling chapter (practices 171-184):

• Throw exceptions instead of returning special values or setting flags
• Make failed builtins throw exceptions too
• Make failures fatal in all contexts
• Be careful when testing for failure of the system builtin
• Throw exceptions on all failures, including recoverable ones
• Have exceptions report from the caller's location, not from the place where they were thrown
• Compose error messages in the recipient's dialect
• Document every error message in the recipient's dialect
• Use exception objects whenever failure data needs to be conveyed to a handler
• Use exception objects when error messages may change
• Use exception objects when two or more exceptions are related
• Catch exception objects in most-derived-first order
• Build exception classes automatically
• Unpack the exception variable in extended exception handlers

See Also

• Ten Essential Development Practices article by Damian Conway: Essential practice No. 8: Throw Exceptions Instead of Returning Special Values or Setting Flags - more detail in the Perl Best Practices book, Chapter 13, Error Handling

• Re^3: What technical benfits perl offers over python + few more questions. by ikegami (2021) - Perl has exceptions, and that's nothing new. Exceptions are thrown using die, and caught using eval BLOCK
• New built-in perl5.38 try/catch syntax by me (2023) - sample code using Perl's latest built-in try/catch syntax

• Try Catch syntax - experimental feature added in perl 5.34
• Try::Tiny by Karen Etheridge
• TryCatch by Ash Berlin

• Re^2: [RFC] Review of module code and POD by Bod (April 2 2021 ... not April 1 ;-)
• Re: Best practices for handling errors by me (2014)
• Re^3: What technical benfits perl offers over python + few more questions. (Try Catch) by me (2021)

• DBI RaiseError attribute (PrintWarn, PrintError, RaiseWarn, RaiseError, HandleError, ...) - DBI's approach to error handling influenced Bod during design

• Re^2: SQL: Update column(s) value with extra WHERE by Marshall (2023) - advises using DBI RaiseError attribute
• Re^3: SQL: Update column(s) value with extra WHERE by bliako - ... prefers to check the error code of each DBI function call
• Re^8: STDERR in Test Results by bliako - ditto
• Re^5: SQL: Update column(s) value with extra WHERE (Real-time Computing and Virtual Machine References) by me - performance is almost never a reason to avoid exceptions in C++ (TODO: what is performance cost of throwing exceptions in Perl?)

• Re: eval to replace die? (Exceptions and Error Handling References)

Updated: added more references to the See Also section.

Have exceptions report from the caller's location, not from the place where they were thrown

I think this is the only one that I'd strongly disagree with.

If you get an error thrown deep in some module as a result of a call, it is really easy to use Carp::confess to find out which line of your own code triggered it. However when you know only the line of your own code that triggered it, it can be far harder to diagnose what has actually gone wrong - if you can't immediately determine it based on the documentation of the call you made, you really want to find the line of code that raised the exception and work back from there.

I'm sure hv knows this, but for others: if you want to make a die in another module to actually confess, use Devel::Confess, e.g.

perl -d:Confess myfailingscript.pl
[download]

I similarly urge you to get into the habit of thinking long and hard about your module's error handling well before you release it, and for the same reasons.

You'll be pleased to hear that I thought at length about error handling before I wrote Business::Stripe::WebCheckout. A consistent approach has been carried over into my other Stripe modules:
Business::Stripe::Subscription
Business::Stripe::Webhook

AFAICT, your basic error handling strategy is for your methods to set the error property

Almost...

The modules set error in any error. The modules provide the success method which returns true on success or false if there was an error. The user of the module is expected to check success. The error method returns the error property so that the user can understand why success is false.

These modules are designed to be used on a webserver. As such, calling die is not helpful to anyone. The end user, the person browsing the website, will probably get an HTTP 500 error and the user of the module is unlikely to detect that quickly.

That's what I meant by: "However, I will not call die. I find it frustrating when modules die"

Instead, I opted for the default error handing approach of DBI. I use this module in default mode (i.e. with RaiseError false) and only turn on RaiseError for debugging, not for live code.

I'm not sure my Stripe modules warrant the equivalent of RaiseError in them. I may be wrong here.

Specifically with Business::Stripe::Webhook, this is designed for building scripts to listen for calls from Stripe to a webhook endpoint. Stripe specifies that the endpoint MUST give a JSON encoded response. The module provides a method for doing this. If the payload from Stripe is invalid or has been changed by the user of the module, then Stripe still needs this reply. If we were to call die then Stripe will not get the reply and may invalidate the endpoint. So not calling die is a very deliberate design decision.

I think a clear statement of your overall error-handling strategy, combined with a couple of real-world examples of handling common errors you've experienced when using your module, would be invaluable to your users

Thank you for this, I shall include it in the next release of each of the Stripe modules.

I work on my own and build modules that are for my use. They only get released if I think they will also be is use for other people. But, like everyone, I suffer from what I call "the curse of knowledge". It is obvious to me what it is supposed to do and how to use it. So it is very difficult to know which bit to explain in full detail and which only need a little explanation.

The modules set error in any error. The modules provide the success method which returns true on success or false if there was an error. The user of the module is expected to check success. The error method returns the error value so that the user can understand why success is false.

I see most of your Business::Stripe::Webhook module's methods begin by clearing the module error property via:

$self->{'error'} = '';
[download]

presumably to avoid confusion from a spurious error property set by an earlier method call. Is that right?

I also noticed your get_subscription method does not do this. Is this a bug?

In case it helps, another minor issue I noticed is the return value of your check_signature method is not documented. Examining the code indicates it can return 1 or 0 or undef. It would be good to document this method's return value and the difference between returning 0 and undef.

presumably to avoid confusion from a spurious error property set by an earlier method call. Is that right?

Spot on...

I also noticed your get_subscription method does not do this. Is this a bug?

Well spotted, yes it is a bug. It has been corrected in my code locally. Although I suppose I should have done that on GitHub!

In case it helps, another minor issue I noticed is the return value of your check_signature method is not documented

It helps a lot thank you.
As it says in the documentation, there shouldn't be any need to call this method. Originally I was going to have it as a private method but decided that there may be a use case for checking Stripe signatures outside of the workflow that's documented. So, in the end it became a public method.

The return values have now been added to POD in my local copy.

G'day Bod,

Here's a few things that I do which you may find useful (possibly in code outside the Business::Stripe:: namespace). In general, these are intended to promote consistency and reduce coding effort.

Have a small number of routines to output messages (e.g. print_error(), print_warning() and so on). These accept a format for the message along with a variable number of arguments. Here's a very rough, off-the-top-of-my-head example:

sub print_error {
    my ($fmt, @args) = @_;
    die 'ERROR: ', sprintf $fmt, @args;
}
[download]

Define each format once only. This could be globally as a constant, specific to a subroutine as a state variable, or using some other method; you may even use a combination of these. Here's some arbitrary examples of formats:

'Start date (%s) cannot be after end date (%s)'
'Postcode (%s) contains invalid characters'
'ID is a required field'
[download]

You can paste these formats directly into the DIAGNOSTICS (or equivalent) section of your POD. See perldiag for a multitude of examples of this type of documentation.

Where possible, I try to collect as many problem reports as possible into a single message. So, if the above three example formats were needed, the user would know to make changes to date, postcode and ID fields from a single piece of feedback, rather than being drip-fed one problem at a time.

How messages are presented will very much depend on the interface. With GUIs (web or other) I often find a small [i] button can provide information about what exactly is required in a field. A popup panel can list the problems and also contain the same [i] buttons for ease of reference.

— Ken

Very helpful, thanks Ken

I have been trying (not always successfully) to have a small number (preferably one) to output everything...this includes a single method as a wrapper around the process method of Template.

It's reassuring to read your advice which is generally aligned with what I'm striving to do.

This reminds me of GraphQL, which is heavily orientated towards usage in web APIs, specifically JSON. See https://graphql.org/ for more, and https://graphql.org/swapi-graphql/?query=%7B%0A%20%20allFilms%20%7B%0A%20%20%20%20films%20%7B%0A%20%20%20%20%20%20title%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D for the Star Wars GraphQL API with a sample query (there's autocomplete in that).

The return values in that are canonically (in JavaScript terms) objects, i.e. in Perl terms hash-refs, which have at least one of data and/or errors. It does exception-catching to turn those into errors entries.

I mention this because it is "guaranteed" (with the usual caveats) to return JSON. I would suggest your web service should adopt a similar philosophy, so that 500s aren't possible, and an appropriate return to the user always happens.