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

🐛 API error when saving entry #6116

Open
bradystroud opened this issue Jan 11, 2022 · 25 comments · Fixed by #6504
Open

🐛 API error when saving entry #6116

bradystroud opened this issue Jan 11, 2022 · 25 comments · Fixed by #6504
Labels
area: api echoes/effort: S Low effort changes (M * 0.5) exp: beginner good first issue hot pinned type: bug code to address defects in shipped code

Comments

@bradystroud
Copy link
Contributor

bradystroud commented Jan 11, 2022

cc: @JackDevAU @pierssinclairssw

Hi,

Describe the bug
Some of our users are getting an error when they try to save an entry.

Failed to persist entry: API_ERROR: Not Found

image
Figure: Error when saving

After some investigation, it looks like the error occurs when Netlify CMS tries to compare the cms changes branch on the users fork with the main branch on the base repository.
e.g. https://api.github.com/repos/SSWConsulting/SSW.Rules.Content/compare/main...JackDevAU:cms/JackDevAU/SSW.Rules.Content/rule/appointments-do-you-show-all-the-necessary-information-in-the-subject/rule

the JackDevAU:cms/JackDevAU/SSW.Rules.Content/rule/appointments-do-you-show-all-the-necessary-information-in-the-subject/rule branch was not created. It is not clear why (see video).
The results are the same when using an incoginto window in a different browser.

The problem is resolved after updating the users fork. To try reproduce the problem, we reverted a fork to an old commit, then made a change, but the entry still saved.

To Reproduce
We cannot consistently reproduce this error as it is only happening to some of our users. See our video attached below demoing the problem.

Expected behavior
The entry should save without error.

Applicable Versions:

  • Netlify CMS core version: 2.54.0
  • Netlify CMS app version: 2.15.59
  • Git provider: GitHub

CMS configuration
https://github.com/SSWConsulting/SSW.Rules/blob/main/src/cms/config.js

Additional context
SSWConsulting/SSW.Rules#801

netlify-api-error-video-final.mp4

Figure: Demo of the error

@bradystroud bradystroud added the type: bug code to address defects in shipped code label Jan 11, 2022
@drwharris
Copy link

drwharris commented Feb 2, 2022

This is a becoming a major issue and makes it almost pointless using Netlify CMS.

@adamcogan
Copy link

This was so painful. Please fix the problem.... or improve that error message please 😢

  • That video explains the problem well.

  • The solution for me was:
    Github.com | My Profile | Your repositories | Select the forked repo eg. SSW.Rules.Content
    Click Fetch upstream
    Click Fetch and merge

     Note: This updates my fork to be usable. When I went back to my edit, I refresh and lost my changes. So I re-did 
    

-a
www.adamcogan.com

@erezrokah
Copy link
Contributor

Hi all and thank you for reporting the issue.

It seems there's a new API to sync forks, see https://docs.github.com/en/rest/reference/branches#sync-a-fork-branch-with-the-upstream-repository

As I'm understanding the issue correctly, we should call that API before saving entries to resolve the issue?
If so, would anyone be open to contribute this fix?

@bradystroud
Copy link
Contributor Author

cc: @JackDevAU
Thanks @erezrokah 🙂, as we aren't familiar with the codebase, could you provide some insight on where this change will need to be added?

@erezrokah
Copy link
Contributor

Sure @bradystroud, the relevant code is here:
https://github.com/netlify/netlify-cms/blob/7bd80b29a43943e5e01a0d7c75573ce02c97f9a3/packages/netlify-cms-backend-github/src/implementation.tsx#L286

We can use the forkExists check and sync the repo if the condition is true.
We can probably optimize that code to not call the forks API too

@asheerrizvi
Copy link
Contributor

asheerrizvi commented Jul 3, 2022

@erezrokah are you looking for something like this?

async authenticateWithFork({
    userData,
    getPermissionToFork,
}: {
    userData: User;
    getPermissionToFork: () => Promise<boolean> | boolean;
}) {
    // ...rest of the code for authenticateWithFork

    if (await this.forkExists({ token })) {
      return fetch(`${this.apiRoot}/repos/${userData.login}/${this.originRepo}/merge-upstream`, {
        method: 'POST',
        headers: {
          Authorization: `token ${token}`,
        },
        body: JSON.stringify({
          branch: this.branch,
        }),
      });
    } else {
      await getPermissionToFork();

      const fork = await fetch(`${this.apiRoot}/repos/${this.originRepo}/forks`, {
        method: 'POST',
        headers: {
          Authorization: `token ${token}`,
        },
      }).then(res => res.json());
      this.useOpenAuthoring = true;
      this.repo = fork.full_name;
      return this.pollUntilForkExists({ repo: fork.full_name, token });
    }
}

I am also seeing this typescript error down the file, not sure what to make of it:

image

@bradystroud
Copy link
Contributor Author

Hey @asheerrizvi
Looks good - Go ahead and create a PR 🙂

@asheerrizvi
Copy link
Contributor

Here's the PR, let me know if you guys need any changes: #6504

@erezrokah @bradystroud

😄

@airtonix
Copy link

ping: needs to be resolved.

@piers-sinclair
Copy link

This is a huge cause of pain for us, @martinjagodic could you please review and action the PR from @asheerrizvi

@martinjagodic
Copy link
Member

We are aware of this problem, we will take a look at the PR soon after we establish the first Decap release. Thanks for your patience 🙏

@adamcogan
Copy link

Ping... what is the status of this one?
-a
www.adamcogan.com

@JeanThirion
Copy link

Definitely not fixed - had the same issue a few times recently :(

@sethdaily
Copy link

Have seen this issue come up many times recently. Would love to have this resolved!

@martinjagodic
Copy link
Member

This issue is in our attention. We are waiting for the PR owner to solve merge conflicts. If he doesn't respond we will do that on a new PR.

@RMunschie92
Copy link

ping: any updates on this?

@martinjagodic
Copy link
Member

The PR is in progress, more details are there. In essence, builds are failing and I was struggling to create a good repoduction. I hope to find some time soon to continue with this.

Any help with the PR would speed things up.

@martinjagodic
Copy link
Member

This is now released in 3.1.0-beta.1. Can anyone test and confirm that this is solved?

@RMunschie92 @sethdailyssw @JeanThirion @adamcogan @pierssinclairssw @airtonix

@bradystroud
Copy link
Contributor Author

Can't tell if its fixed because there is a new bug
#6924

@sethdaily
Copy link

I still get the error when clicking 'save' after I've made a change, see screenshot:
image
Figure: Same error

@AntPolkanov
Copy link

AntPolkanov commented Jan 15, 2024

I am still having the same issue on save.
It has been 2 years... It's so painful... 😭
image

@martinjagodic
Copy link
Member

@bradystroud I see that many of you are from SSW. Could you find some time together to contribute?

@bastien-germain
Copy link

Up, is there any update on this issue ?

@demshy
Copy link
Member

demshy commented Apr 4, 2024

not really I'm afraid

I'm willing to put some effort into this if we find a hero who would describe a very specific scenario on how to (somewhat consistently) reproduce this issue (maybe with a public repo?)

@mohab-sameh
Copy link

@demshy Commenting as I had several people I know report the same thing. Somehow it got resolved by deleting and re-inviting the user in Netlify identity. Willing to help with testing but can't consistently reproduce it. However, it's a major blocker when it happens..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api echoes/effort: S Low effort changes (M * 0.5) exp: beginner good first issue hot pinned type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging a pull request may close this issue.