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

Properly handling 'just unfrozed' requests #10

Closed
willnode opened this issue Mar 18, 2018 · 3 comments
Closed

Properly handling 'just unfrozed' requests #10

willnode opened this issue Mar 18, 2018 · 3 comments

Comments

@willnode
Copy link
Contributor

willnode commented Mar 18, 2018

In some cases gh-latest-repos returns just an empty request [].

My hypothesis of cause

I observe in now that a frozen microservice means the process just been killed and only be activated again if a request are made, which is a problem because variables are lost and request to GitHub API have to be reinitiated again.

Because variables are lost, there's race condition between client request and request to GitHub API, hence gh-latest-repos just returns [] instead of waiting for response from GitHub API.

And because of #9 I realized that I can't make a workaround by requesting to gh-latest-repos twice. 😢

Solution

I have my own patch to send 202 (Accepted) without cache control in case empty [] will send (so the client can make another request again)

if (responseText === '[]') {
	response.statusCode = 202;
	response.end('[]');
	return;
}

I can made a PR for this but I realize this is too much for just solving this problem so I have a better idea:

Make fetchRepos() to be synchronous?

@sindresorhus
Copy link
Owner

sindresorhus commented Mar 18, 2018

What I've done previously to solve this kind of issue is to make responseText a Promise instead. A promise is only resolved once, so we just always await it. If it's ready, it's served right away, if not, it causes the connection to wait until it's ready. When fetchRepos updates, we just assign the responseText variable a new Promise.

@willnode
Copy link
Contributor Author

@sindresorhus that is good idea. Though I'd actually never heard Promises before but will try anyway and see if the solution works 👍

@sindresorhus
Copy link
Owner

This is no longer relevant. The code was rewritten for Now v2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants