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.

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

Tweak Specs2 CatsResource to rethrow exceptions raised during resource acquisition #608

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

bpholt
Copy link
Member

@bpholt bpholt commented Nov 20, 2024

Basically, if an exception is raised when acquiring the val resource: Resource[F, A], it would previously be swallowed by the Future launched by beforeAll. Due to the exception, the gate is never closed (i.e., the Deferred is never completed), so the Deferred#get call in withResource never completes.

This PR puts a timeout on the Deferred#get call in withResource to avoid waiting forever. It also updates var value to maybe contain Either[Throwable, A], so that the exception raised in the acquisition phase is raised in the test, so that the user can see the details of the exception and hopefully resolve the underlying issue.

@bpholt bpholt force-pushed the resource-acquisition-exception branch from 8e59882 to 8ce3b4f Compare November 20, 2024 19:59
When using CatsResource with Specs2, if the resource acquisition raises
an exception, the tests will never start and the exception will never
be reported.
@bpholt bpholt force-pushed the resource-acquisition-exception branch from 8ce3b4f to 4a70db2 Compare November 20, 2024 21:08
@bpholt bpholt mentioned this pull request Nov 22, 2024
@bpholt bpholt requested a review from armanbilge November 26, 2024 18:46
Copy link
Member

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

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

Great catch!

@djspiewak djspiewak merged commit 1c903c0 into typelevel:series/1.x Nov 27, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants