Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ResponseCancelTest #2297

Closed
Scottmitch opened this issue Jul 29, 2022 · 3 comments · Fixed by #2337
Closed

ResponseCancelTest #2297

Scottmitch opened this issue Jul 29, 2022 · 3 comments · Fixed by #2337
Assignees
@idelpivnitskiy
Labels
flaky tests Unit tests are flaky

Comments

@Scottmitch
Copy link
Member

https://github.com/apple/servicetalk/runs/7573293427

io.servicetalk.transport.netty.internal.StacklessClosedChannelException
	at io.servicetalk.transport.netty.internal.DefaultNettyConnection.channelInactive(...)(Unknown Source)

possibly related: #952
@Scottmitch Scottmitch added the flaky tests Unit tests are flaky label Jul 29, 2022
@Scottmitch Scottmitch mentioned this issue Jul 29, 2022
Merged
@idelpivnitskiy idelpivnitskiy self-assigned this Jul 29, 2022
@Scottmitch
Copy link
Member Author

https://github.com/apple/servicetalk/runs/7769023858

ResponseCancelTest.clientCancel()
io.servicetalk.transport.netty.internal.StacklessClosedChannelException

io.servicetalk.transport.netty.internal.StacklessClosedChannelException
	at io.servicetalk.transport.netty.internal.DefaultNettyConnection.channelInactive(...)(Unknown Source)

@Scottmitch Scottmitch mentioned this issue Aug 10, 2022
Merged
@Scottmitch
Copy link
Member Author

https://github.com/apple/servicetalk/runs/8083042325

ResponseCancelTest.connectionCancel()

o.servicetalk.transport.netty.internal.StacklessClosedChannelException
	at io.servicetalk.transport.netty.internal.DefaultNettyConnection.channelInactive(...)(Unknown Source)

@Scottmitch Scottmitch mentioned this issue Aug 30, 2022
Merged
@Scottmitch Scottmitch changed the title ResponseCancelTest.connectionCancel() ResponseCancelTest Aug 30, 2022
@Scottmitch
Copy link
Member Author

snapshot build failure after updating to Netty 4.1.80
https://github.com/apple/servicetalk/runs/8083545803

ResponseCancelTest.clientCancel()
io.servicetalk.transport.netty.internal.StacklessClosedChannelException
	at io.servicetalk.transport.netty.internal.DefaultNettyConnection.channelInactive(...)(Unknown Source)


ResponseCancelTest.connectionCancel()
io.servicetalk.transport.netty.internal.StacklessClosedChannelException
	at io.servicetalk.transport.netty.internal.DefaultNettyConnection.channelInactive(...)(Unknown Source)

@idelpivnitskiy idelpivnitskiy mentioned this issue Aug 30, 2022
Merged
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this issue Aug 31, 2022
@idelpivnitskiy
Fix flaky ResponseCancelTest
5759aa5 
Motivation:

apple#2297 indicates that `ResponseCancelTest` sometimes fails. It happens
because we incorrectly request signals from `delayedClientTermination`
queue. After cancel we may or may not see a terminal event. In rare
cases, `StacklessClosedChannelException` is propagated to the subscriber
after cancel. The next request does not assume that there is a pending
`ClientTerminationSignal` in the queue and considers this exception as
failure for a new request.

Modifications:
- Introduce a `requestId` to associate `ClientTerminationSignal` with
a proper request;
- Discard signals for prior requests inside `resume` logic;
- Wrap `signal.err` with `AssertionError` to preserve a caller stack
trace;

Result:

Fixes apple#2297.
@idelpivnitskiy idelpivnitskiy mentioned this issue Aug 31, 2022
Merged
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this issue Sep 28, 2022
@idelpivnitskiy
Fix flaky ResponseCancelTest
e07cdb3 
Motivation:

apple#2297 indicates that `ResponseCancelTest` sometimes fails. It happens
because we incorrectly request signals from `delayedClientTermination`
queue. After cancel we may or may not see a terminal event. In rare
cases, `StacklessClosedChannelException` is propagated to the subscriber
after cancel. The next request does not assume that there is a pending
`ClientTerminationSignal` in the queue and considers this exception as
failure for a new request.

Modifications:
- Introduce a `requestId` to associate `ClientTerminationSignal` with
a proper request;
- Discard signals for prior requests inside `resume` logic;
- Wrap `signal.err` with `AssertionError` to preserve a caller stack
trace;

Result:

Fixes apple#2297.
idelpivnitskiy added a commit that referenced this issue Sep 29, 2022
@idelpivnitskiy
Fix flaky ResponseCancelTest (#2337)
906c4fc 
Motivation:

#2297 indicates that `ResponseCancelTest` sometimes fails. It happens
because we incorrectly request signals from `delayedClientTermination`
queue. After cancel we may or may not see a terminal event. In rare
cases, `StacklessClosedChannelException` is propagated to the subscriber
after cancel. The next request does not assume that there is a pending
`ClientTerminationSignal` in the queue and considers this exception as
failure for a new request.

Modifications:
- Introduce a `requestId` to associate `ClientTerminationSignal` with
a proper request;
- Discard signals for prior requests inside `resume` logic;
- Wrap `signal.err` with `AssertionError` to preserve a caller stack
trace;

Result:

Fixes #2297.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@idelpivnitskiy idelpivnitskiy

Labels
flaky tests Unit tests are flaky
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix flaky ResponseCancelTest idelpivnitskiy/servicetalk
2 participants
@idelpivnitskiy @Scottmitch