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

Error message after saving branch [racing condition problem] #157

Open
pameck opened this issue Apr 11, 2017 · 7 comments
Open

Error message after saving branch [racing condition problem] #157

pameck opened this issue Apr 11, 2017 · 7 comments

Comments

@pameck
Copy link
Member

pameck commented Apr 11, 2017

Error Description:

After a branch is created, the following message is displayed:

screen shot 2017-04-12 at 9 15 13 am

In the dev console, there is this:

screen shot 2017-04-12 at 9 15 41 am

To replicate:

  1. Go to the admin dashboard
  2. Create a new branch
  3. See error.

Possible cause:

When a new branch is created, there are two subsequent requests:

GET /branches/{id}/groups
GET /branches/{id}/members

The problem is that we're adding the new branch to the core's store after it has gone through the event pipe, lambdas and back to core, but the requests from the UI get to the core before the new branch is added to the store which is why those two requests get a 404 error.

The following diagram is for reference about the branch creation flow:

branches-with-events 1

@pameck pameck added the bug label Apr 11, 2017
@yearofthedan
Copy link
Contributor

It's an interesting problem. A couple of options come to mind.

  • The front end could use the information it has for the branch rather than making a request to the backend after saving. This would mean the FE should set the id in the initial create request for future use.
  • We could wait a fixed time before requesting the branch, but that feels quite hacky.
  • We could poll the backend for the data after we save and then enforce a timeout. This would be a little chattier.

@camjackson
Copy link
Contributor

camjackson commented Apr 18, 2017

Wait, so, why is the frontend even making those requests? I don't think it's an ID generation issue, because the backend generates the ID before putting the event on the stream, and it will return that ID to the frontend synchronously.

From @pameck's screenshot, it seems the frontend is trying to fetch the new branch's groups and members, which is probably unnecessary. I think it's safe to assume that a brand new branch doesn't have any groups or members yet, so those fields can just be initialised to empty arrays, rather than fetching empty arrays from the backend.

So proposed solution: just don't make those requests 😁

@yearofthedan
Copy link
Contributor

I havent looked too deep, but I think the issue is that the app selects the newly created branch which triggers a new API call for the branch details. So that would mean either not selecting, or creating some new flow.

@pameck
Copy link
Member Author

pameck commented Apr 18, 2017

I think that on top of the change in the UI, the API should return an empty array instead of a 404 when asking for a collection of things.

@yearofthedan
Copy link
Contributor

But also you're right; ignore what I said about IDs. I had a blip and forgot that the create request returns one.

@yearofthedan
Copy link
Contributor

Even if the branch can't be found?

@pameck
Copy link
Member Author

pameck commented Apr 18, 2017

@yearofthedan right! It's the branch that's missing not the groups or members, back to the 404 😬

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

No branches or pull requests

3 participants