After creating releases wait for GitHub to propagate changes #32
Conversation
dgellow
changed the title
src/github.ts
Outdated
| private invalidateFileCache() { | ||
| this.fileCache = new RepositoryFileCache(this.octokit, this.repository); | ||
| } |
There was a problem hiding this comment.
release-please github client uses RepositoryFileCache from git-file-utils, a simple read-through cache for a repository. But there is currently no way to invalidate entries or easily bypass the cache, so we need to recreate the whole object.
(I opened a feature request here)
src/github.ts
Outdated
| ); | ||
| return; | ||
| } | ||
| await sleepInMs(100 * attempt); |
There was a problem hiding this comment.
Naïve backoff just to avoid sending requests too quickly, I don't think we need anything more fancy here but I may be wrong.
| } catch (e: unknown) { | ||
| // other errors are already retried by octokit-plugin-retry | ||
| if (e instanceof FileNotFoundError) { | ||
| notFoundError = e; | ||
| this.logger.warn( | ||
| `Failed to fetch ${filePath} on branch ${branch}`, | ||
| notFoundError | ||
| ); | ||
| } else { | ||
| throw e; | ||
| } | ||
| } |
There was a problem hiding this comment.
@marcel-stainless I just thought about it, we already handle other errors via octokit/plugin-retry. The only one we need to handle here would be the file-not-found one.
There was a problem hiding this comment.
I will merge the PR as it is, tell me if you want me to rollback or change the error handling here, I can follow-up with a small PR
dgellow
deleted the
sam/sta-2269-post-apitrigger-release-please-created-unwanted-release-pr
branch
Ok, this one was a real pain to debug and find a reliable way to deal with, I approached it way too optimistic! But it is an important fix to make the release creation more reliable and avoid a whole set of issues that would (and currently does) impact end users.
In short, after creating github releases it takes a little bit of time for new/updated data to be available through GitHub's API. We need to wait a bit before assuming data returned by the API are up to date.
In more details, this pull request implements two mitigations for conditions that occur when chaining multiple release-please commands (in Stainless case, running
createReleases()directly followed bycreatePullRequests()).octokit.repos.listReleases()nexton top ofmainto be sure fetching the manifest returns the updated versionNote that we did not face this problem before reading the manifest from
next, I believe this change is what created conditions for this issue.Something else to note is that this pull request in itself doesn't fix the whole issue, we also need changes to be sure we aren't using outdated data when re-using the same manifest object for multiple commands. I have a quick fix for this for the monorepo.
In case you're curious, that's the type of logs I get, from what I've seen it takes 1-3s before
nextcontent reflects the branch re-alignment: