Merge grpc headers and trailers in case of failure

[NavidJalali]

Attempt to fix #392

[pjfanning]

A lot of unit tests are broken by this change.

[NavidJalali]

@pjfanning yes, I am investigating them now. I didn't run the tests when I was checking if it fixed my issue.

[NavidJalali]

Ran into an unrelated issue:
apache/pekko-http#622

[raboof]

Ran into an unrelated issue: apache/pekko-http#622

That's strange, where does that Trailer without headers get created?

[pjfanning]

I would not favour merging this. It ties us to an unreleased version of pekko-http - and I think there is an argument that the real issue is in the AWS code.

[raboof]

I would not favour merging this. It ties us to an unreleased version of pekko-http - and I think there is an argument that the real issue is in the AWS code.

I agree we should not merge this as long as it depends on an unreleased version of Pekko HTTP. The PR is still in 'Draft' mode, so that tracks ;).

I agree there's a case to be made that the 'real issue' of #392 is in the AWS implementation. That said, responding with a Strict response for errors like a 404 instead of streaming (which is the core of this PR) seems like a reasonable optimization regardless.

It might still be possible to adapt this PR to avoid creating the situation where a Trailers object without any headers is created, so it can go back to using the regular released Pekko HTTP version. In any case it'll still take a couple of rounds of improvements to make this change nice and ready to merge, but I'm optimistic about the chances of getting there.

[raboof]

@NavidJalali thanks for working on this, let me know if you'd like me to review your work-in-progress!

[pjfanning]

For practical reasons, it would be good to change this PR to avoid creating Trailers with no headers.

[NavidJalali]

Ran into an unrelated issue: apache/pekko-http#622

That's strange, where does that Trailer without headers get created?

I had to dig to find it, but it seems to be created by some fallback logic I removed in this commit

[NavidJalali]

Looks like I missed something, I'll get back to debugging

[raboof]

I like the general direction this is going, a couple of review points!

[raboof]

This looks good!

(the link validator failure is unrelated, #395)

[NavidJalali]

@pjfanning is the link validator flakey? would it be green if you ran it again? I would love to get a snapshot build

[pjfanning]

@NavidJalali don't worry about the link validator - we tend to ignore the results of that build - see #395

[pjfanning]

lgtm