Skip metadata processing after sending local reply#16154
Skip metadata processing after sending local reply#16154yanavlasov merged 3 commits intoenvoyproxy:mainfrom
Conversation
|
Hi @GinYM, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
yanavlasov
left a comment
yanavlasov
left a comment
There was a problem hiding this comment.
Can you add unit and also integration test if possible for this new case, please?
|
/wait |
Sure, will do that. |
GinYM
force-pushed
the
early-stop
branch
2 times, most recently
from
b5085b1 to
9846ad2
Compare
There was a problem hiding this comment.
I think this test is the same as the DownstreamProtocolIntegrationTest.ContinueAfterLocalReply and it does not test the change you have made. To test you change the decodeHeaders() of your test filer should return StopAll and then the decodeMetadata() should just have ASSERT(false); in it. I think the exception should work too, but it would be harder to diagnose than ASSERT.
And the test should just test for response from local reply. If it hits the ASSERT in the decodeMetadata then it would fail.
There was a problem hiding this comment.
Thanks for review. Your are right, I have added StopIteration in decoderHeaders and removed the EXPECT_ENVOY_BUG. Aslo replace throw with assert.
|
/wait |
|
/retest |
|
Retrying Azure Pipelines: |
|
LGTM, you will need to resolve merge conflict before I can merge. |
|
/wait |
Signed-off-by: Yiming Jin <jinyiming1996@gmail.com>
…ply. Signed-off-by: Yiming Jin <jinyiming1996@gmail.com>
Signed-off-by: Yiming Jin <jinyiming1996@gmail.com>
Done. |
|
/retest |
|
Retrying Azure Pipelines: |
|
/retest |
|
Retrying Azure Pipelines: |
Signed-off-by: Yiming Jin <jinyiming1996@gmail.com>
3 participants
Signed-off-by: Yiming Jin jinyiming1996@gmail.com
Commit Message: Skip metadata processing after sending local reply
Fixes #15654