parseVersion2 throws away underlying reader error

[emersion]

When parseVersion2 gets an error from the bufio.Reader, it throws it away. This makes it tricky to recover the source failure on network error.

Instead, it should wrap the underlying error with its own.

[pires]

Can you, please, implement a test that proves this?

[emersion]

This is a pattern all over the place in the library parsers. Here's a test which trips one spot:

func TestReadError(t *testing.T) {
	raw := append(append(SIGV2, byte(PROXY), byte(TCPv4)), fixtureIPv4V2...)
	wantErr := errReadIntentionallyBroken

	// r returns an error starting from the 17th byte
	r := io.MultiReader(bytes.NewReader(raw[:17]), &errorReader{})
	br := bufio.NewReader(r)
	_, err := Read(br)
	if err != wantErr {
		t.Errorf("Read() = %q, want %q", err, wantErr)
	}
}

[pires]

I understand better now what you mean. Wrapping errors is indeed the right thing to do but it will break how consumers of the API check for errors, eg err == proxyproto.ErrCantReadProtocolVersionAndCommand. While breaking, I do think it's something we should do. WDYT?

[emersion]

Indeed, I don't see a way to implement this without breaking the API. I would agree that it's worth addressing still.

[pires]

Please, test and review #165.