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

fix: handle disconnected streams on codegen download #395

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

pvlugter
Copy link
Member

Refs #394

Switch over to axios, which gets the disconnected signals. And use stream.pipeline which adds error handling to the stream automatically (otherwise it's adding event handlers for error and aborted on the response data, and close on the writer for resolving successfully).

This just logs the error currently, on disconnected streams. Looks like this

Fetching kalix-codegen-js from https://repo.lightbend.com/raw/kalix/versions/1.0.0/kalix-codegen-js-x86_64-apple-darwin
Saving to .../kalix-scripts/bin/kalix-codegen-js.bin
Failed to download Kalix codegen binary: Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
    at new NodeError (internal/errors.js:322:7)
    at IncomingMessage.onclose (internal/streams/end-of-stream.js:117:38)
    at IncomingMessage.emit (events.js:400:28)
    at TLSSocket.socketCloseListener (_http_client.js:432:11)
    at TLSSocket.emit (events.js:412:35)
    at net.js:686:12
    at TCP.done (_tls_wrap.js:564:7) {
  code: 'ERR_STREAM_PREMATURE_CLOSE'
}

Would make sense to wrap in some retries.

@github-actions github-actions bot added the kalix-runtime Runtime and SDKs sub-team label Jul 13, 2022
@pvlugter pvlugter force-pushed the download-codegen branch 2 times, most recently from e8afdf0 to 15198f8 Compare July 18, 2022 04:33
@pvlugter pvlugter changed the title wip: disconnected streams on codegen download fix: handle disconnected streams on codegen download Jul 18, 2022
@pvlugter
Copy link
Member Author

The underlying issue with the proxy load balancing seems to be resolved, but it may still be useful to have the error handling here. Not sure we should worry about retries, but it does at least detect and show the error, and remove the partial download so it can't be used in a broken way.

@pvlugter pvlugter marked this pull request as ready for review July 18, 2022 04:37
@pvlugter
Copy link
Member Author

Also add a process exit on errors, so that scripts don't continue past this point and it's clear it failed.

Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.

@ennru ennru merged commit 17b88f3 into lightbend:main Aug 3, 2022
@pvlugter pvlugter deleted the download-codegen branch August 8, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kalix-runtime Runtime and SDKs sub-team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants