Atomic Batching

[jogboms]

I wrote a simple example to verify async behaviour with the batch implementation and noticed the following behaviour.

import 'package:signals/signals.dart';

final age = signal(0);
final doubleAge = computed(() => age() * 2);

void main() async {
  effect(() {
    print('listen-age: $age');
  });

  age.set(2);

  batch(() async {
    age.set(0);

    await Future.delayed(const Duration(seconds: 1));

    age.value += 1;

    print('inner-double-age: $doubleAge');

    age.value += 1;
  });

  print('double-age: $doubleAge');
}

And this is the output:

listen-age: 0
listen-age: 2
listen-age: 0
double-age: 0
listen-age: 1
inner-double-age: 2
listen-age: 2

Is this expected behaviour? I have not yet looked into the batching but my first guess is it doesn't cater for async logic and that the context is shared.

[jogboms]

Its not just an async-related behaviour. In this case, even though the batch operation fails, age is mutated to 0, this could be an undesired state.

import 'package:signals/signals.dart';

final age = signal(0);
final doubleAge = computed(() => age() * 2);

void main() {
  effect(() {
    print('listen-age: $age');
  });

  age.set(2);

  batch(() {
    age.set(0);

    throw Exception('error');
  });

  print('double-age: $doubleAge');
}

[rodydavis]

First of all signals are only sync and the graph is meant only for synchronous operations.

In a batch it is not a transaction and will update any values till it has an error.

This is how other signal libraries work too

[jogboms]

In a batch it is not a transaction and will update any values till it has an error.

Agreed. Any chance this can make it to the docs? I noticed the preact version also works in the exact same way with no support for transactions.

[rodydavis]

Yep happy to add it to the docs!

[rodydavis]

Going to work on a section in the docs with approaches to async with signals. But updating some of my production apps and already learning some common patterns

[Solido]

Started a discussion here on Rollback #108
We can keep the idea of transaction by monitoring all signals inside a specific batch.
When an exception is thrown, rollback all previously executed signals.
Not planed but an idea to keep in mind for the future ^^ we possibly can if we accept
that integrity is more important than perfs.

[rodydavis]

What would be the behavior for signals with no buffer?

[Solido]

Obviously they need ‘real’ Tx that we don’t support. Throw an Exception would be default or batch can Be explicitly configured to silence incompatible Signals and failed rollback. Dev opt-in Tx behavior nothing magic happens. On paper seems possible. See any holes?

…

On Tue 9 Jan 2024 at 23:16, Rody Davis ***@***.***> wrote: What would be the behavior for signals with no buffer? — Reply to this email directly, view it on GitHub <#105 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ4MWPQSZWMQ7WQFCPCJCTYNW6TBAVCNFSM6AAAAABBGL7BKCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBTHA4DQOJUHE> . You are receiving this because you commented.Message ID: ***@***.***>

[jinyus]

This would be hard/impossible to implement because in real apps, batch callbacks wouldn't be pure functions and would include async ops and signals can't track anything after an async gap. Maybe a pausing mechanism could achieve this.

mySignal.pauseUpdates();

try{
    var newValue = await someNetworkCall()
   // do some more work to compute the new value
}finally{
    mySignal.resume();
   // if an exn is thrown, we don't even have to rollback because it didn't change
}

mySingal.value = newValue;

This could be done in an atomicBatch() function that takes a dependency array of signals to pause.

Just an idea.

[rodydavis]

I think another solution could be this:
https://github.com/maverick-js/signals?tab=readme-ov-file#onerror

[jinyus]

That looks like it would only work for synchronous code.

I think the effect would have to catch the error and pass it to the onError callback...but effect wouldn't catch an async error, unless you're ok with it being limited to sync code.

[rodydavis]

That is correct it would only work for sync code.

Another option is to improve the batch API. Current it is the following:

T batch<T>(BatchCallback<T> callback) {

We could add a Rollback callback that would only be called on error:

T batch<T>(
     BatchCallback<T> callback,
+   [BatchCallback<T>? rollback],
   ) {
  if (_batchDepth > 0) {
    return callback();
  }
  _startBatch();
  try {
    return callback();
+  } catch (e) {
+   final result = rollback?.call();
+    if (result is T) return result; 
+    rethrow;
  } finally {
    _endBatch();
  }
}

Something like that. And it would be backwards compatible.

Current will not rollback:

batch(() {
  count.value = 1;
  throw;
});

if rollback is provided it will be called on error:

batch(() {
  count.value = 1;
  throw;
}, () {
  count.value = 0;
});

It could also be a named param too.

[rodydavis]

This would not work for async code, but for sync signal this would be a nice way to rollback if needed.

[rodydavis]

Been thinking about this more, I think it would be better to use dart exception handling for this:

import 'package:signals/signals.dart';

final age = signal(0);
final doubleAge = computed(() => age() * 2);

void main() {
  effect(() {
    print('listen-age: $age');
  });

  age.set(2);

  batch(() {
+ try {
    age.set(0);
    throw Exception('error');
+   } catch (e) {
+ age.set(2); // or age.set(age.previousValue);
+ }
  });

  print('double-age: $doubleAge');
}