Add status_message_with_fallback GH#105,#114

[robrwo]

No description provided.

[oalders]

Thanks @robrwo. I see a merge conflict. Maybe you need to pull some commits into your branch?

[vanHoesel]

Hi @robrwo ,

I will write a -hopefully- better approach for this problem space than the new 'default go to' function you introduced.

See the issue #114, where I suggest the introduction of status_class_code function that will generate the five nXX strings (which then would need to be added to the %StatusCode hash).

This will make things more explicit, rather than introducing a potentially default go to function that will always come with some message string.

Status Codes are super fundamental stuff, you should not just skip but be fully aware that there is something going wrong with whatever passed into it. With a simple default function, one would not even know that somebody passed rubbish, Being explicit gives you the chance to issue warnings.

[robrwo]

@vanHoesel I disagree with you here. The class of a status code indicates roughly what the response is, even if the user agent doesn't know how to handle a specific status code.

It is entirely fine to issue any status code between 100 and 599. Just because upstream software (e.g. PSGI or Net::Server) doesn't recognise it, or it's not in an RFC, doesn't mean that there is something wrong.

Unfortunately, a lot of modules relay on there being a non-undef/non-blank status message, which means that they either have to implement their own fallbacks for handling unknown status codes, or we can provide something that gives the authors a standard way to handle these gracefully.

Adding fake status codes to the hash seems more likely to break things, or require rewriting the is_methods etc.

[vanHoesel]

Why is this not "Continue' ?

Suggested change

	is_info( $_[0] ) ? 'OK'
	is_info( $_[0] ) ? 'Continue'

[vanHoesel]

As mentioned in a new proposal in #114, I'd like to suggest to rename this new function to status_message_fallback that will only give a string back if the code is not listed yet.

This way, there is no DWIM default go to method that encourages people to write code that becomes insanely hard to debug.

The proposed solution will allow one to write:

my $message = status_message($code);
unless ($message) {
    $message = status_message_fallback($code);
    croak "Status code not understood [$code], assuming ($message)";
}

or

my $message = status_message($code) || status_message_fallback($code);

and

my $message = status_message_fallback(200) or die;

will just do that

[vanHoesel]

Continue ?

Suggested change

	is(status_message_with_fallback(199), "OK");
	is(status_message_with_fallback(199), "Continue");

[vanHoesel]

I created a pull request: robrwo#1 that will do the status_message_fallback as I proposed ... So, Rob can have the major credites as he did come up with the problem space and wrote the tests etc. Only thing for @robrwo would be to press [MERGE] button on his forked repo

[oalders]

@vanHoesel, if we merge your branch into this one, is this ready to be merged into master? (Aside from the merge conflict?)

[vanHoesel]

@oalders , I will need to have a look again, myself. A good thing to clean up stale issues and merge request.

Let me get back to this

[vanHoesel]

Please help me reason about the behaviour of status_message_fallback it returning an explicit undef when the code is not one of the 5 known classes. The POD on this new function seems to be incomplete, as it does not mention it would need to be used in scalar context and that it does return a undef 'value'.

Once those are agreed upon, I can make a final merge request.

[robrwo]

It's been a couple of years, and if I recall, there was some disagreement w.r.t. requested changes. I'll revisit this in the next few days, when I have the time.