Revert "Simplify log-wait-strategy (#977)"

[cristianrgreco]

This reverts commit b837e90.

Fixes #982.

I raised a PR #983 which cancels the timeout if the wait strategy completes, but less risky to just revert. FYI @danielbodart

[netlify]

✅ Deploy Preview for testcontainers-node ready!

Name	Link
🔨 Latest commit	e210b5d
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-node/deploys/67f7a423e25ea700082d8888
😎 Deploy Preview	https://deploy-preview-984--testcontainers-node.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[danielbodart]

Hey @cristianrgreco did you manage to find a way to test this or easily repo it?

Just I'd argue that the probably the better change would have been to allow the docker-container-client.logs method to take a signal, this would be more inline with how you do things in say the http-wait-strategy which then passes the signal into fetch.

Then you could just delete the Promise.race and timeout completely and just call AbortSignal.timeout when calling the logs method.

When I look at the logs method, my reading is that you are calling unref to try and make sure it can again shut down, is that the correct read? Seems like if we changed this to use a signal we might potentially simplify that as well. Even if not that it would put all the logic in one place which is probably better.

Just checked an dockerode supports passing in an abortSignal

Another thought, you mentioned before about Jest tests and shutting down, that might be a good case for calling destory in a finally block