Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Propagate start errors. #94

Merged
merged 3 commits into from
Jun 28, 2022
Merged

Propagate start errors. #94

merged 3 commits into from
Jun 28, 2022

Conversation

T4rk1n
Copy link
Contributor

@T4rk1n T4rk1n commented Jun 28, 2022

Propagate start errors instead of the same Address already in use error messages when starting the threaded server.

@T4rk1n T4rk1n requested a review from alexcjohnson June 28, 2022 15:34
pass
req = requests.get(alive_url)
res = req.content.decode()
if req.status_code == 500:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two questions here:
(1) Any chance we'd fail with a code other than 500? Maybe switch to if req.status_code != 200?
(2) Is there any way an error would make it into the queue later, ie during the requests.get call? Should we check the queue again after receiving this 500 response (or even check always after requests.get?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(1) The error handler always return 500, that catches the errors in the before_first_request in dash and layout/deps errors. I don't think in dash we return other status code, but there could be with plugin.
(2) The queue get the error from server start, the requests error goes into Exception handler to return a 500, if the server failed to start the requests.get will raise connection error, next retry it get queue error if it wasn't there. Think if there a requests error it should check the queue after to ensure no race condition to get the proper error on the last retry.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 LGTM!

@T4rk1n T4rk1n merged commit 86e5746 into master Jun 28, 2022
@T4rk1n T4rk1n deleted the fix-start-error branch June 28, 2022 17:24
@benedikt-budig benedikt-budig mentioned this pull request Jan 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants