Make pushing a new scope atomic.

[kodhi-tech-lead]

When pushNewScope is called with the init function provided, throw an exception if there is an attempt to push another scope during the execution of init. This is useful when delegating to individual sub-modules the registration of objects to a scope defined/controlled by the parent.

Thank you.

[escamoteur]

sounds like an absolute reasonable request. will implement in the next release

[kodhi-tech-lead]

Thank you.

[escamoteur]

done in upcoming V8

[kuhnroyal]

I added a comment to the commit here: https://github.com/fluttercommunity/get_it/commit/afa6dd3c39b68d852934be80f0540119b1e0a9a3#r147433011

But adding here as well for easier tracking:

If there is an error thrown during scope push, the _pushScopeInProgress variable needs to be reset to false. Right now it is possible to lock the whole instance if there is an error thrown and never get it working again. Maybe additionally reset the flag in reset()?

[escamoteur]

I didn't know that is is possible to add a comment to any commit. Interesting. That opens another question, what would be the correct behavior in case the the init call throws? Remove the scope again? Can the app get into any stable state in that case at all?

[kuhnroyal]

I didn't know that is is possible to add a comment to any commit. Interesting. That opens another question, what would be the correct behavior in case the the init call throws? Remove the scope again? Can the app get into any stable state in that case at all?

That is a great question and I am not sure that there is an easy answer. The only thing that is clear to me, is that a reset should clear the flag.

[escamoteur]

Where would you call the reset then? Or is it just to make sure not all tests fail after a failed pushScope?

[kuhnroyal]

Yes, I noticed this in a failing test case, so this is a use case. But I can also imagine showing some kind of error page and a "Try again" button that resets everything.

[escamoteur]

If we see the pushing of a new scope as something that consists of creating the new scope but also the successful finish of the init function, the correct behavior is probably to remove the scope and throw an error

[kuhnroyal]

Remove the scope again?

After thinking about this for a couple minutes, I think this is the best approach. Removing the scope and all its registrations. For my usage, I push new scopes for different navigator routes. If the scope push fails, I can deny the navigation and allow the user to try again or do whatever. At least the app is in some semi-predictable state.

If it is left as is, it is not predictable at all.

[escamoteur]

does this look good?

    _pushScopeInProgress = true;
    _scopes.add(_Scope(name: scopeName, disposeFunc: dispose));
    try {
      init?.call(this);
      if (isFinal) {
        _scopes.last.isFinal = true;
      }
      onScopeChanged?.call(true);
    } catch (e) {
      final failedScope = _scopes.last;

      /// prevend any new registrations in this scope
      failedScope.isFinal = true;
      failedScope.reset(dispose: true);
      _scopes.removeLast();
      rethrow;
    }
    finally {
      _pushScopeInProgress = false;
    }

[kuhnroyal]

I think so!

[kuhnroyal]

If you want, I can create a PR and add a test

[escamoteur]

let me push my change and please, add a test, that would be awesome

[escamoteur]

pushed

[kuhnroyal]

@escamoteur I create a PR with the tests and also 2 other PRs with small improvements.

[kuhnroyal]

@escamoteur Is there anything missing to get my PRs merged and this fix possibly released?

[escamoteur]

Oh, sorry, no I was just too busy. Let me check later today. Am 21. Okt. 2024, 16:07 +0100 schrieb Peter Leibiger @.***>:

@escamoteur Is there anything missing to get my PRs merged and this fix possibly released? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[escamoteur]

merged and published V8.0.1 thanks a lot

[kuhnroyal]

Thanks!

[escamoteur]

Really appreciated your contribution. If you want to help maintaining get_it you are more than welcome 😁 Am 22. Okt. 2024, 09:47 +0100 schrieb Peter Leibiger @.***>:

Thanks! — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>