Cleanup artifact handlers hanging node process #1596
Conversation
| const zipUploadStream = await createZipUploadStream( | ||
| zipSpecification, | ||
| options?.compressionLevel | ||
| ) | ||
|
|
There was a problem hiding this comment.
This has just been moved down, right?
There was a problem hiding this comment.
Yea, we were initializing this wayyy above in the method before we were even consuming the stream
| debug(`[Response] - ${response.message.statusCode}`) | ||
| debug(`Headers: ${JSON.stringify(response.message.headers, null, 2)}`) | ||
| debug(`Body: ${body}`) |
There was a problem hiding this comment.
I believe we may need to remove these lines.
There was a problem hiding this comment.
I actually added them to assist in debugging customer workflows. If they run into an issue, they can turn on debug logs and this can potentially help figure out what's wrong
|
|
||
| if (this.isSuccessStatusCode(statusCode)) { | ||
| return response | ||
| return {response, body} |
There was a problem hiding this comment.
This is changing the return type; are there any tests failing?
There was a problem hiding this comment.
This method is only used internally by the twirp client. However I do want to revisit this retry logic eventually and add more tests.
| @@ -80,11 +80,13 @@ class ArtifactHttpClient implements Rpc { | |||
| try { | |||
| const response = await operation() | |||
| const statusCode = response.message.statusCode | |||
| debug(`[Response] ${response.message.statusCode}`) | |||
| debug(JSON.stringify(response.message.headers, null, 2)) | |||
| const body = await response.readBody() | |||
Fixes a bug where an error response causes the actions to hang for ~2 min.
Using
why-is-node-runningwe were able to track down theZLIB,TCPWRAP, andTLSWRAPhandlers clogging things up.tl;dr