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

[Bug]: Unexpected behavior when running same API request multiple times #53

Closed
jonas-martinez opened this issue Oct 28, 2022 · 2 comments · Fixed by #55
Closed

[Bug]: Unexpected behavior when running same API request multiple times #53

jonas-martinez opened this issue Oct 28, 2022 · 2 comments · Fixed by #55
Assignees
Labels
bug Something isn't working

Comments

@jonas-martinez
Copy link
Collaborator

jonas-martinez commented Oct 28, 2022

What happened?

It seems that our current API request system using the models produces an unexpected behavior when running the same API request multiple times (no matter the parameters as long as the Status variable is the same for the requests).

This must be due to the way we handle request statuses. In fact we use a class variable for the Status which means that if two same requests run concurrently it will try to update the same Status variable. The first request to finish will be ok but the second will just not run the request and fetch the result from the first (because the status was updated to done).

A possible solution

Define the Status variables inside of the method that runs the request. This will ensure that two requests cannot edit the same Status at the same time.
This means that we must change where we use the status to observe the state of the request to use the Future's result instead. This should not be a problem.

Before

class Example {
  Status<GetMainEnvResponse> getMainEnvStatus = Status();
  
  Future<GetMainEnvResponse> getMainEnv(int appId) async {
      var res = await getMainEnvStatus.handle(
        () => ApplicationApi.getMainEnv(appId),
        notifyListeners,
      );
      mainEnv = res.mainEnv;
      notifyListeners();
      return res;
    }
}

After

Future<GetMainEnvResponse> getMainEnv(int appId) async {
    Status<GetMainEnvResponse> getMainEnvStatus = Status();
    var res = await getMainEnvStatus.handle(
      () => ApplicationApi.getMainEnv(appId),
      notifyListeners,
    );
    mainEnv = res.mainEnv;
    notifyListeners();
    return res;
  }
@jonas-martinez jonas-martinez added the bug Something isn't working label Oct 28, 2022
@jonas-martinez
Copy link
Collaborator Author

We could still use the Status as a class variable for the requests that will not be run simultaneously anyway. Or can be run simultaneously and will not alter the final result.

@jonas-martinez
Copy link
Collaborator Author

After some discussions with @taorepoara we could just use a Map with the appId as key and the status as value for the example shown above. And replicate this solution where we need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants