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

Leaked socket in cache/internal/cacheHttpClient #1702

Open
mweberxyz opened this issue Apr 4, 2024 · 0 comments
Open

Leaked socket in cache/internal/cacheHttpClient #1702

mweberxyz opened this issue Apr 4, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@mweberxyz
Copy link

actions/cache#1217 attempted to work around a dangling socket, which became especially apparent when that action migrated to Node 20, as Node v20 started keeping connection alive by default.

This solution worked for actions/cache in the meantime, but caused downstream consumers of to implement the same logic to avoid hanging processes and generated similar issues, such as:

The leaked socket originates in cache/src/internal/cacheHttpClient:

  const uploadChunkResponse = await retryHttpClientResponse(
    `uploadChunk (start: ${start}, end: ${end})`,
    async () =>
      httpClient.sendStream(
        'PATCH',
        resourceUrl,
        openStream(),
        additionalHeaders
      )
  )

sendStream return a response, the body of must be consumed to avoid holding the event loop open.

Something like the following ought to do it:

  const uploadChunkResponse = await retryHttpClientResponse(
    `uploadChunk (start: ${start}, end: ${end})`,
    async () => {
      const response = await httpClient.sendStream(
        'PATCH',
        resourceUrl,
        openStream(),
        additionalHeaders
      )

      await response.readBody()

      return response
    }
  )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant