Either or throw?

[SimoneBressan]

Il problema secondario ma non risolto perchè è un BrakingChange è in produzione, questo approccio alle use case basato sugli Either porta ad alcuni problemi:

• Sulla Failure bisogna mantenere lo StackTrace del punto di creazione della failure che se non è necessaria non serve e questo è molto pesante in termini di performance
• La gestione dell'Either e' complicata e prolissa anche quando dovrebbe essere molto semplice

Lanciare le Failure con throw porterebbe a notevoli vantaggi, le failure non gestite andrebbero direttamente sul logger e verebbe creato lo stackTrace solo quando il logger riceve l'errore inoltre i tipi sarabbero sicuri e la scrittura del codice verebbe piu breve e leggibile.

Either:

1. 🔴 Più posti dove viene gestita la failure con lo stesso codice, si puo ovviare scrivendo estensioni apposta per i futuri
2. 🔴 E' necessario controllare il tipo della failure con il costrutto if ( is ) per ogni failure
3. 🔴 Se non si vuole gestire la failure bisogna inviarla al logger ottenendo cosi uno stackTrace astruso
4. 🟢 E' possibile utilizzare "facilmente" la use case con il pacchetto provider ma richiede una scrittura prolissa per il recupero del valore final res = context.read<Either<Failure, Success>>() e poco usabile/usato lato presentation
   ThrowCatch:
5. 🟢 Un unico posto dove gestire la failure
6. 🟢 Non è necessario controllare il tipo della failure dato che è presente la keywork on
7. 🟢 Se non si vuole gestire la failure non serve far nulla, viene catturata dal logger con lo stackTrace corretto e chiaro
8. 🟢 Non si ottiene codice anidadato o codice prolisso con molti controlli di tipo
9. 🔴 Non sempre è possibile passare avanti nella catena l'errore, l'utilizzo diretto con il pacchetto provider non è possibile. E' veramente necessario? No, dovresti passare sempre per il bloc appena la logica diventa complicata la quale necessita l'utilizzo di provider

Sono mostrati solo esempi sicuri da scrivere, che non possono presentare errori di tipo
Con Either:
Esempio 1:

final result = await myUseCase.call(MyUseCaseParams());

result.fold((failure) {
  if (failure is SpecificFailure) {
    // Handle my specific failure
    return;
  } 
  // Handle a failure
}, (myRes) async {
  final result = await myUseCase.call(MyUseCaseParams());

  result.fold((failure) {
    // Stesso codice della precedente gestione della failure
  }, (myRes) {
    // Hendle result
  })
})

Esempio 2:

final result = await myUseCase.call(MyUseCaseParams());

if (result is Left<Failure, MyResultType>) {
  final failure = result.value;
  if (failure is SpecificFailure) {
    // Handle my specific failure
    return;
  } 
  // Handle a failure
  return;
}
final result = await myUseCase.call(MyUseCaseParams());

if (result is Left<Failure, MyResultType>) {
  // Stesso codice della precedente gestione della failure
  return;
}

// Handle all results

Con throw e catch
Esempio 3:

try {
  final result = await myUseCase.call(MyUseCaseParams());
  final result = await myUseCase.call(MyUseCaseParams());
} on SpecificFailure {
  // Handle my specific failure
} on Failure {
  // Handle failure
  // Un unico posto dove gestire la failure
}

• Ci sono altri vantaggi o svantaggi nell'uso di uno di questi due approcci?
• Per esperienza personale quale di questi approcci è stato più corretto e con meno bug?

Link esterni:
provider
bloc

Altri problemi conosciuti sono stati presentati in parte in questa PR #12.
Come dovresti far per recuperare la UI da un errore in aspettato con i diversi casi di utilizzo?
Con Either: Esempio 1:

try {
  final result = await myUseCase.call(MyUseCaseParams());
  
  result.fold((failure) {
    if (failure is SpecificFailure) {
      // Handle my specific failure
      return;
    } 
    // Handle a failure
  }, (myRes) async {
      // Hendle result
  })
} catch (error) {
   // Recupera la UI in caso fosse necessario
  retrhow; // Invia l'errore al logger perchè non gestito
}

Con throw e catch: Esempio 2:

try {
  final result = await myUseCase.call(MyUseCaseParams());
} on Failure {
  // Handle failure
} catch (error) {
  // Recupera la UI in caso fosse necessario
  retrhow; // Invia l'errore al logger perchè non gestito
}

Non è molto chiaro mischiare i due casi di gestione dell'errore per quanto riguarda l'esempio 1 invece nell'esempio 2 rimane tutto molto chiaro

[kuamanet]

TL;DR

I'm not sure you are using Either with the right intent

Either reasons

Either is used to ensure that whoever wants to access a value handles a possible error with at least a no-op.

It's not a matter of how simple is to read the actual value, or how handy is the stacktrace but a matter to be sure that errors are described and handled.

From your examples:

try {
  final result = await myUseCase.call(MyUseCaseParams());
  
  result.fold((failure) {
    if (failure is SpecificFailure) {
      // Handle my specific failure
      return;
    } 
    // Handle a failure
  }, (myRes) async {
      // Hendle result
  })
} catch (error) {
   // Recupera la UI in caso fosse necessario
  retrhow; // Invia l'errore al logger perchè non gestito
}

This is not the right way to use an either. If somebody is returning you an either, is explicitly assuring you that the call will not throw. So this should just be

final result = await myUseCase.call(MyUseCaseParams());
  
  result.fold((failure) {
    if (failure is SpecificFailure) {
      // Handle my specific failure
      return;
    } 
    // Handle a failure
  }, (myRes) async {
      // Hendle result
  })

On failure handling

final result = await myUseCase.call(MyUseCaseParams());

if (result is Left<Failure, MyResultType>) {
  final failure = result.value;
  if (failure is SpecificFailure) {
    // Handle my specific failure
    return;
  } 
  // Handle a failure
  return;
}
final result = await myUseCase.call(MyUseCaseParams());

if (result is Left<Failure, MyResultType>) {
  // Stesso codice della precedente gestione della failure
  return;
}

// Handle all results

This is not the way Either is intended to be used. This is the way you should be using it:

// somewhere..
void errorHandle(Failure error) {
 if (failure is SpecificFailure) {
    // Handle my specific failure
    return;
  } 
  // Handle a failure
  return;
}
final result = await myUseCase.call(MyUseCaseParams());

result
.map(() => await myOtherUseCase.call(MyUseCaseParams()) // or flatMap or whatever...
.fold(errorHandler, (value) => {
   // do something with the value
})

Moreover, Either should encourage you modelling and declaring your internal exceptions.

On this:

try {
  final result = await myUseCase.call(MyUseCaseParams());
} on Failure {
  // Handle failure
} catch (error) {
  // Recupera la UI in caso fosse necessario
  retrhow; // Invia l'errore al logger perchè non gestito
}

Nobody ensures you that somebody will use your api ( myUseCase.call ) without a try/catch

// Possible reasons
// I'm a lazy person that does not read your documented throws
// I'm a noobie and nobody told me that if I don't try/catch this block the world will end
final result = await myUseCase.call(MyUseCaseParams());

Basically, you are making examples of wrong usages.

1. A call to a method that returns an Either should not be inside a try/catch. If you need to try/catch the call, 100% the method you are calling has a bug
2. Either, as most of functional constructs, can be composed with other Eithers, no need to create a "callbacks hell"

In the end, if you don't feel comfortable using this construct as "final consumer" of the resulting apis it is not a problem, I just want to be sure that you get the right reason behind the usage of Either.

Using Either your intent is to expose

• a set of described and well known errors
• a set of described and well known values
• a set of calls that do not throw

If you don't need these behaviours because your are handling exceptions somewhere else in your code, Either is not the right path for you.

[SimoneBressan]

@kuamanet Non mi è chiaro l'intento che vuoi promuovere con questo commento.

E' chiaro l'intento di Either "Obbligarti a gestire le failure" ma poi nel codice potresti non gestirle lo stesso.
Non vedo benefici sostanziali nel continuare ad usare Either.
La perdita dello stackTrace e l'aumento della complessità del codice (anche con quanto hai appena spiegato) sono mancanze molto più gravi dei "benefici" che porta

PS: Either non permette di conoscere la lista degli errori che riceverai. Come il throw. Deve essere scritto nella documentazione

[SimoneBressan]

Non mi è chiara questa risposta.

La discussione dovrebbe vertere su quali dei due approcci sono meglio su dart se throw o Either

Mi hai spiegato cosa fa e come si usa e come dovrebbe essere usato Either ma nulla di piu. Non è pertinente al 100% con il quesito che ho posto

[kuamanet]

La perdita dello stackTrace....

Basarsi sullo stack trace non e' certamente una best practice. Ma soprattutto, l'utilizzatore di either non deve avere interesse allo stacktrace, deve solo gestire come meglio crede i diversi tipi di errori.

Either non permette di conoscere la lista degli errori che riceverai. Come il throw.

Non e' vero. Dipende da come lo scrivi.

class BaseError {} 
class Error1 extends BaseError {}
class Error2 extends BaseError {}

Either<BaseError, String> either; // so esattamente l'elenco di possibili errori

e l'aumento della complessità del codice

try {
  final result = await myUseCase.call(MyUseCaseParams());
} on Failure {
  // Handle failure
} catch (error) {
  // Recupera la UI in caso fosse necessario
  retrhow; // Invia l'errore al logger perchè non gestito
}

// vs

final result = await myUseCase.call(MyUseCaseParams());
result.fold(errorHandler, successHandler)

Di preciso dov'e' l'aumento di complessita'?

Either non e' in alternativa al throw. Either serve

1. ad imporre la gestione dell'errore all'utilizzatore, nel senso che il codice non compila se non gestisci anche il left, a differenza del throw.
2. a sgraviare l'utilizzatore del pensiero "Ma questo codice throwa? E che cosa? E perche'?". L'utilizzatore sa che o ottiene il valore, o ottiene un errore che descrive (per convenzione) esattamente qual'e' il problema.

connectToBleDevice.call().fold(onConnectionError, onDeviceConnected)

....
void onConnectionError(ConnectionFailure f) {// so esattamente quali possono essere le failures, non serve che vado a leggermi cosa fa la usecase. Chi l'ha scritta ha descritto in ConnectionFailure (e sue sottoclassi) i vari motivi di fallimento dell'azione
  if(f is MissingBluetoothPermission) {
      ....
  } else ...
}

In particolare

try {
  final result = await myUseCase.call(MyUseCaseParams());
} on Failure {
  // Handle failure
} catch (error) {
  // Recupera la UI in caso fosse necessario
  retrhow; // Invia l'errore al logger perchè non gestito
}

Questo sembra piu' codice che dovrebbe stare dentro una usecase

Il quesito che poni (Either or throw) e' sbagliato alla base. Non sono due modi alternativi di fare la stessa cosa.

[kuamanet]

A me sembra che stai spostando quello che dovrebbe essere dentro ad una usecase in altri posti. Le usecase non sono solo un bridge 1 a 1 tra il data layer e il presentation layer.

La domanda al massimo puo' essere "Come vogliamo gestire gli errori?"

1. Non ci interessa, speriamo che vengano try/catchati
2. Vogliamo essere sicuri che vengano gestiti

Io tendenzialmente preferirei la 2.

[SimoneBressan]

@kuamanet

Basarsi sullo stack trace non e' certamente una best practice. Ma soprattutto, l'utilizzatore di either non deve avere interesse allo stacktrace, deve solo gestire come meglio crede i diversi tipi di errori.

Si vero. Ma siamo anche sviluppatori di quella libreria e ci interessa conosce da dove proviene una failure in caso di debug.
Ancora di piu se vogliamo trasformare i null pointer excepition in failure. Altrimenti non lo dovremmo mai fare, ma solo ritornare failure che veramente stiamo gestendo all'interno della use case

Non e' vero. Dipende da come lo scrivi.

Questo lo puoi fare anche throw. Non è un valido argomento

Allora l'implementazione attuale delle use case è sbagliata. Dovresti avere una Failure base per ogni UseCase
Quindi non dovresti mai dover gestire failure condivise tra più use case

Di preciso dov'e' l'aumento di complessita'?

Questo dovrebbe essere

try {
  final result = await myUseCase.call(MyUseCaseParams());
} on Failure {
  // Handle failure
} catch (error) {
  // Recupera la UI in caso fosse necessario
  retrhow; // Invia l'errore al logger perchè non gestito
}

// vs

void errorHandler(failure) {
   // Handle failure
}

void successHandler(result) {
  // ...
}

final result = await myUseCase.call(MyUseCaseParams());
result.fold(errorHandler, successHandler)

Principalmente la complessità la si ritrova quando devi fare più either uno dentro l'altro, in successione

Questo sembra piu' codice che dovrebbe stare dentro una usecase

E' soggettivo. Dipende soltanto dal paradigma che siamo consoni da usare

Vogliamo essere sicuri che vengano gestiti

Anche se usi Either potresti ottenere

final either = ...;
either.fold((failure) {
  // Non faccio nulla
}, (success) {...})

Quindi anche in questo caso non li stai gestendo
Però non hai modo di risalire allo stackTrace così

[SimoneBressan]

Vediamo qualcosa di più complesso. Prova a scrivere questo con l'Either

  fn() async {
    try {
      final documentGroupFB = <int, GroupDocumentFieldBloc>{
        KycFormSteps.idDocument: idDocumentFB,
        KycFormSteps.proofOfAddress: proofOfAddressFB,
        KycFormSteps.selfie: selfieFB,
      }[state.currentStep]!;
      final documentsFB = documentGroupFB.documentsFB;
      final countryFB = documentGroupFB.countryFB;

      // Upload any documents from this current step
      for (final docFB in documentsFB.value) {
        final file = docFB.value!;
        final data = docFB.state.extraData!;

        // Not upload already uploaded file
        if (file.mimeType == 'fake') continue;

        try {
          await _uploadUserDocumentUC.call(UploadUserDocumentParams(
            file: file,
            type: data.type,
            subType: data.subType,
            country: countryFB.value,
          ));
        } on Failure catch (failure, stackTrace) {
          // File is big
          if (failure is RequestTooLargeFailure) {
            docFB.addFieldError('Request to large', isPermanent: true);
            emitFailure();

            lg.w({
              '${state.currentStep} Failure on upload document ${data.type}.${data.subType}': {
                'fileName': file.name,
                'Size': await file.length()
              }
            }, failure, stackTrace);
            return;
          }
          rethrow;
        }
      }
      // If current step is a last step
      // Mark product completed and open a wallet
      if (state.currentStep == KycFormSteps.selfie) {
          await _acquireKycProductUC.call(ProductId.account);
      }
      
      emitSuccess();
    } on Failure catch (failure) {
      emitFailure(failureResponse: failure);
    }
  }

[SimoneBressan]

Sinceramente io non lo so scrivere con Either senza usare is sull'Either dato che aumenta notevolmente la complessità del codice

[kuamanet]

Si vero. Ma siamo anche sviluppatori di quella libreria e ci interessa conosce da dove proviene una failure in caso di debug.
Ancora di piu se vogliamo trasformare i null pointer excepition in failure.

ci interessa internamente, non all'esterno. Perche' dovremmo trasformare un null pointer exception in failure? null pointer exception e' un bug da risolvere, non un'eccezione da gestire e presentare all'esterno

Altrimenti non lo dovremmo mai fare, ma solo ritornare failure che veramente stiamo gestendo all'interno della use case
esatto

Allora l'implementazione attuale delle use case è sbagliata. Dovresti avere una Failure base per ogni UseCase
Quindi non dovresti mai dover gestire failure condivise tra più use case

Sicuramente avere set di usecase che non hanno failure con classi basi comuni aiuterebbe la gestione dell'errore

E' soggettivo. Dipende soltanto dal paradigma che siamo consoni da usare

usecase == businesslogic, non e' proprio super soggettivo

Anche se usi Either potresti ottenere

final either = ...;
either.fold((failure) {
// Non faccio nulla
}, (success) {...})
Quindi anche in questo caso non li stai gestendo
Però non hai modo di risalire allo stackTrace così

Sí, posso non voler gestire la failure. Ma scritto cosí é esplicito

either.fold(noop, onSucces)

Scritto cosí non sappiamo se veramente non si voleva gestire l'errore o se ci si e' dimenticati

/// throws Exception1
/// throws Exception2
/// throws ...
void errorProneMethod() {... }

try {
 errorProneMethod()
} on Exception1 {
 ...
}

fn() async {
  final documentGroupFB = <int, GroupDocumentFieldBloc>{
    KycFormSteps.idDocument: idDocumentFB,
    KycFormSteps.proofOfAddress: proofOfAddressFB,
    KycFormSteps.selfie: selfieFB,
  }[state.currentStep]!;
  final documentsFB = documentGroupFB.documentsFB;
  final countryFB = documentGroupFB.countryFB;

  // Upload any documents from this current step
  for (final docFB in documentsFB.value) {
    final file = docFB.value!;
    final data = docFB.state.extraData!;

    // Not upload already uploaded file
    // if (file.mimeType == 'fake') continue; THIS SHOULD BE INSIDE THE USECASE

    final res = await _uploadUserDocumentUC.call(UploadUserDocumentParams(
      file: file,
      type: data.type,
      subType: data.subType,
      country: countryFB.value,
    ));

    res.map(onUploadSuccess).leftMap(onUploadError(docFB));
    // or res.fold(onUploadError(docFB), onUploadSuccess)

    if (res.isLeft()) {
      return; //stop uploading at first error
    }
  }
}

onUploadSuccess(DocumentEntity r) async {
  // If current step is a last step
  // Mark product completed and open a wallet
  if (state.currentStep == KycFormSteps.selfie) {
    await _onSubmittingLastStep();
  } else {
    emitSuccess();
  }
}

Function(Failure) onUploadError(InputFieldBloc<XFile, DocumentFieldBlocData> docFB) {
  return (Failure failure) {
    // ALSO LOGGING SHOULD BE INSIDE THE USECASE LAYER
    /*lg.w({
              '${state.currentStep} Failure on upload document ${data.type}.${data.subType}': {
                'fileName': file.name,
                'Size': await file.length()
              }
            }, res.left);*/
    // File is big
    switch (failure.runtimeType) {
      case RequestTooLargeFailure:
        docFB.addFieldError('Request to large', isPermanent: true);
        emitFailure();
        break;
      default:
        emitFailure(failureResponse: failure);
    }
  };
}

.map sull'either esegue solo se e' right.
Poi dipende volendo probabilmente si riesce a trasformare il loop in un map o qualcosa del genere

[SimoneBressan]

Non fa le stesse cose che ho scritto nel mio codice.

res.map(onUploadSuccess).leftMap(onUploadError(docFB));
Dovresti farlo solo alla fine se tutti i precedenti header hanno avuto successo

A parte che è bruttissimo scritto cosi

Scusa ti aggiungo

void onSubmittingLastStep() {
  try {
      await _acquireKycProductUC.call(ProductId.account);

      emitSuccess();
    } on Failure catch (failure) {
      emitFailure(failureResponse: failure);
    }
}

// ALSO LOGGING SHOULD BE INSIDE THE USECASE LAYER
Questo sarei interessato a vederlo non commentato, per una semplice questione di vedere come gestisci il futuro dentro a un fold che poi dovrai ricontrollare

[kuamanet]

Questo sarei interessato a vederlo non commentato, per una semplice questione di vedere come gestisci il futuro dentro a un fold che poi dovrai ricontrollare

non ho capito scusami.. la parte commentata e' il logging... che c'entra il futuro?

[SimoneBressan]

non ho capito scusami.. la parte commentata e' il logging... che c'entra il futuro?

Per loggare ha bisogno di un valore ritornato da file.lenght() che ritorna un futuro, quindi voglio vedere come gestisce quella parte che io non so come scrivere

[kuamanet]

Non capisco perche' continui a mettere le chiamate alle usecase in un try catch