Don't re-use reaper if it failed to initialize #258
Conversation
| @@ -77,11 +77,13 @@ func NewReaper(ctx context.Context, sessionID string, provider ReaperProvider, r | |||
|
|
|||
| c, err := provider.RunContainer(ctx, req) | |||
| if err != nil { | |||
| reaper = nil | |||
There was a problem hiding this comment.
Hey @kernle32dll, thanks for taking the effort of reporting this!
I see this PR would work although I'm concerned about its maintainability if new code conditions are added in the future.
How would you see delaying the creation of the Reaper struct instance (L51) until the container and the endpoint are created (right before the old L87)? I'd see the code more maintainable in this case. wdyt?
|
@kernle32dll are you still interested in this PR? If not, I'd like to label it as |
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
With merging #782, I believe this PR became obsolete and can be closed? |
|
I believe so. I'm closing it but feel free to reopen if it persists, thanks! |
If the reaper fails to initialize, it leaves testcontainers in a broken state. To be precise, the
Endpointvariable is never properly initialized, which causes subsequent usages of testcontainers to fail withfailed to create container: connecting to reaper failed: Connecting to Ryuk on failed: dial tcp: missing address.