connection.cpp: Check return value of write()

[Lotterleben]

Issue

Resolve one more instance of #1682 .
I manually checked all other instances that

ag '\.write\((.*),(.*)\);' | wc -l
ag '\.read\((.*),(.*)\);' |wc -l

brought up and they were either minor/irrelevant (in examples or tests where this wasn't the focus) or resolved a few lines down (with another if (!result)).

I hope what I did makes sense; if not, I'd of course be happy to improve my PR according to your suggestions.

Tasklist

• CHANGELOG.md entry (How to write a changelog entry) -- If I understand it correctly this doesn't qualify as a change that needs to be mentioned in the changelog
• update relevant Wiki pages -- same
• add tests (see testing documentation -- same
• review
• adjust for comments
• cherry pick to release branch

Requirements / Relations

--

[danpat]

Hmm, while I think it's good to check for error here, I'm not sure that throwing is the right solution - this will kill the server process, as I don't think we have a high-level catch on a per-request basis.

Write errors like this one should cause the client on the other end of the connection to receive an error message, but probably shouldn't cause the osrm-routed process to terminate.

[Lotterleben]

Hmm, while I think it's good to check for error here, I'm not sure that throwing is the right solution - this will kill the server process, as I don't think we have a high-level catch on a per-request basis.

Argh, of course! Sorry about that.

Write errors like this one should cause the client on the other end of the connection to receive an error message, but probably shouldn't cause the osrm-routed process to terminate.

I'm probably missing something, but should the error message really trickle down all the way to the client? If I understand it correctly, compress_buffers() is only trying to apply the compression method set in the HTTP Accept-Encoding header, which, if I understand this description correctly, is more of a suggestion– "if you choose to compress, I understand the following methods". If the server fails to compress, no harm done to the client.

That being said, the compression failure should probably still be logged at the server?

[TheMarex]

Sorry for the belated answer, cleaning up the old PRs. ✨

@Lotterleben My reading of that description is the same, though I think we would need to return plaintext if compression fails, right?

So if anyone feels like picking this up again:

• We need to probably surface to the caller of this function that compression failed (either through the return value or through a custom exception).
• The call code needs to handle that and return plaintext instead.

[DennisOSRM]

closing stale PR. Reopen if still relevant.