Skip to content

Conversation

alaviss
Copy link
Contributor

@alaviss alaviss commented Oct 4, 2025

Summary

Fix intermittent Windows CI failures from an unhandled SslError
being raised due to abrupt peer disconnection in tssl .

Details

The goal of the test is to verify that closing an SSL socket after a
fatal error happens will not cause the action to raise an error.
Sometimes this fatal error manifests as SslError , which was not
handled, causing intermittent failures (on Windows).

This commit modifies the test to also ignore SslError should that
happen, since we are only interested in the close statement not
raising an error.

The goal of the test is to verify that closing a SSL socket
after a fatal error happened will not cause the action to raise
an error. Sometimes this fatal error manifests as `SslError`,
which was not handled, causing intermittent failures (on Windows).

This commit modifies the test to also ignore `SslError` should that
happens, since we are only interested in the `close` statement not
raising an error.
@alaviss alaviss added test Add or improve tests bug Something isn't working labels Oct 4, 2025
@saem saem added the stdlib Standard library label Oct 4, 2025
@saem
Copy link
Collaborator

saem commented Oct 4, 2025

@alaviss so why the sudden increase in frequency of occurrence, is that because of underlying version changes in the CI system or something?

@alaviss
Copy link
Contributor Author

alaviss commented Oct 5, 2025

Seems to me that GH is slowly rolling out OpenSSL 3.x: actions/runner-images#12676

This might be hitting us on upgraded runners.

Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest, I don't trust the actual module or the test, and don't know the ssl api well enough to know if this is right... but a less chirpy CI and the rest of the tests passing seem like a win.

@saem
Copy link
Collaborator

saem commented Oct 5, 2025

/merge

Copy link

github-actions bot commented Oct 5, 2025

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

  • Should solves the various Windows-related CI failures.

@chore-runner chore-runner bot added this pull request to the merge queue Oct 5, 2025
Merged via the queue into nim-works:devel with commit 03d17a9 Oct 5, 2025
44 checks passed
@alaviss alaviss deleted the push-ovsytqklxvnu branch October 5, 2025 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working stdlib Standard library test Add or improve tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants