Change HttpParserError to be subclass of StandardError

[MSP-Greg]

Description

A long time ago (02-April-2006), HttpParserError was added, and it was a subclass of IOError. Today, IOError is more associated with errors at the IO level.

Puma should differentiate between socket level errors and HTTP request validity errors. Hence, change HttpParserError to be subclass of StandardError. This change will stop HttpParserError from being 'swallowed' in:

puma/lib/puma/client.rb

Lines 184 to 190 in f5e4b77

	begin
	if fast_check && @to_io.wait_readable(FAST_TRACK_KA_TIMEOUT)
	return try_to_finish
	end
	rescue IOError
	# swallow it
	end

This PR also uncomments two tests in test/test_request_invalid.rb, which fail on master.

Note that @to_io.wait_readable may throw other socket related errors. Something to keep an eye on...

Closes #3552

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
• My pull request is 100 lines added/removed or less so that it can be easily reviewed.
• If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
• If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
• I have updated the documentation accordingly.
• All new and existing tests passed, including Rubocop.

[dentarg]

Sounds like the right thing to do

[MSP-Greg]

@dentarg

Thanks for the quick review. I'll wait a little while to see if anyone else has comments.

As to a method to my madness (and three open PR's dealing with request error handling), this started with a realization that Puma wasn't properly closing invalid keep-alive requests, and leaving the parser with a buffer full of some or all of request 'A'.

After the error, since it was a keep-alive, the code in Server#handle_request then tried to see if another request was on the socket, and starts by parsing the buffer, which is part of request 'A'. Not pretty.

Then, I noticed that code that checked and/or modified the env was contained in both client.rb and request.rb (which is included in server.rb). That is confusing and also makes CI more involved. The reorg for that is in PR #3582... The code in the client tests is much simpler, I haven't yet written code to allow a 'test' request to be sent in more than one chunk with delays in between...

[joshuay03]

Makes sense to me, nice one!