Open SimoneBressan opened 3 years ago
I'm not sure you are using Either
with the right intent
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.
Either
should not be inside a try/catch. If you need to try/catch the call, 100% the method you are calling has a bugEither
, as most of functional constructs, can be composed with other Either
s, 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
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.
@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
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
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
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.
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?"
Io tendenzialmente preferirei la 2.
@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ì
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);
}
}
Sinceramente io non lo so scrivere con Either senza usare is
sull'Either dato che aumenta notevolmente la complessità del codice
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
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
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?
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
Non capisco perche' continui a mettere le chiamate alle usecase in un try catch
Devo riscrivere il messaggio precedente? che per sbaglio lo ho cancellato
Esempio di use case con try e catch e la failure In questo caso se la chiamata della use case crash con ad esempio un null pointer excepition non ritornerà un Either con la failure ma bensì lancerà un'eccezione come detto precedentemente, dato che è un bug Ma io vorrei comunque, anche se c'è questo bug, ripristinare la ui in un stato valido cosi da poter far riprovare l'utente a rifare la chiamata
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
le call / tryCall sono gia' async, cosa ti disturba?
try {
final doc = await _storageRepo.upload(
file,
CreateDocumentEntity((b) => b
..type = params.type
..subType = params.subType
..country = params.country?.code),
);
} on Error {
lg.w(
{
'${params.currentStep} Failure on upload document ${params.type}.${params.subType}': {
'fileName': file.name,
'Size': await file.length()
}
},
);
}
Ma io vorrei comunque, anche se c'è questo bug, ripristinare la ui in un stato valido cosi da poter far riprovare l'utente a rifare la chiamata
Se la chiamata ha prodotto un null pointer exception non ha senso riprovarci. Riprodurra' un null pointer exception. Tra le varie avrai pure lo stack trace 😅
Per favore riprendi lo stesso codice che ho scritto prima e capirai dove sta il problema senza cambiarne il funzionamento
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);
}
}
Basically the behaviour is
RequestTooLargeFailure
do addFieldError
&& emitFailure
otherwise emitFailure(failureResponse: failure);
selfie
do _acquireKycProductUC
right?
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
var somethingFailed = false;
for (final docFB in documentsFB.value) {
final res = await _uploadUserDocumentUC.call(fieldBlocToParams(docFB));
res.leftMap(onUploadError(docFB));
somethingFailed = res.isLeft();
if (somethingFailed) {
break;
}
}
if (!somethingFailed && state.currentStep == KycFormSteps.selfie) {
final res = await _acquireKycProductUC.call(ProductId.account);
res.leftMap(...) // possibly handle this uc failures
somethingFailed = res.isLeft();
}
if (!somethingFailed) {
emitSuccess();
}
}
Function(Failure) onUploadError(InputFieldBloc<XFile, DocumentFieldBlocData> docFB) {
return (Failure failure) async {
switch (failure.runtimeType) {
case RequestTooLargeFailure:
docFB.addFieldError('Request to large', isPermanent: true);
emitFailure();
break;
default:
emitFailure(failureResponse: failure);
}
};
}
UploadUserDocumentParams fieldBlocToParams(InputFieldBloc<XFile, DocumentFieldBlocData> docFB) {
return UploadUserDocumentParams(
file: docFB.value!,
type: docFB.state.extraData!.type,
subType: docFB.state.extraData!.subType,
country: countryFB.value,
);
}
Ripeto, hai saltato parti del codice Comunque è più complicato del mio. Meno funzionale del mio e con più magia
Cioè hai gli either mi aspetterei che utilizzassi quelli per controllare se ha fallito o no, non avere secondi variabili.
Questo è molto "magico", non vorrei mai dover scrivere questo, inoltre non puoi scriverlo questo pezzo di codice Tutte i metodi di Either non supportano async Il vero quesito è come mettere un futuro dentro ad un metodo dell'Either senza dover scrivere estensioni ad hoc
Function(Failure) onUploadError(InputFieldBloc<XFile, DocumentFieldBlocData> docFB) {
return (Failure failure) async {
switch (failure.runtimeType) {
case RequestTooLargeFailure:
docFB.addFieldError('Request to large', isPermanent: true);
emitFailure();
break;
default:
emitFailure(failureResponse: failure);
}
};
}
Either<L2, R> leftMap<L2>(L2 f(L l)) => fold((L l) => left(f(l)), right);
Ripeto, hai saltato parti del codice Comunque è più complicato del mio. Meno funzionale del mio e con più magia
Ma che parti??
Piu' complicato di un try catch nested con un rethrow??
Function(Failure) onUploadError(InputFieldBloc<XFile, DocumentFieldBlocData> docFB) {
return (Failure failure) async {
quell'async e' inutile , si puo togliere, mi e' rimasto li
Il vero quesito è come mettere un futuro dentro ad un metodo dell'Either senza dover scrivere estensioni ad hoc
tipo un leftMapAsync
o foldAsync
?
si
Ma che parti??
lg.w({
'${state.currentStep} Failure on upload document ${data.type}.${data.subType}': {
'fileName': file.name,
'Size': await file.length()
}
}, failure, stackTrace);
return;
Non abbiamo detto che va nella usecase?
Vorrei capire come dovrei gestire delle use case annidate, dato che ritornano futuri e Either non li supporta
Probabilmente il casino è quello, dovremmo restituire un Either<Future<L>, Future<R>>
. Provo a giocarci e ti aggiorno
@SimoneBressan Per concatenare le use case attualmente abbiamo bisogno del set di estensioni create da ResoDev qui
Con quelle e' possibile fare il flatMap/map/leftMap delle varie usecase. Li puoi usare con la consapevolezza che map su un either esegue solo se l'either e' un right, quindi nell'esempio di upload potresti mappare l'array di documenti e mettere un flatMap per eseguire qualcosa solo quando tutte le usecase vanno a buon fine e un leftMap all'errore. Domani provo a scrivertela bene.
L'alternativa e' creare una usecase che aggrega piu' usecase.
Non è un po troppo complicato? Sarebbe molto semplice se utilizzassimo try e catch
XD se il problema è "è troppo complesso mi rallenta lo sviluppo" ok, ma allora stiamo discutendo del nulla da 2 giorni :D
Io sto discutendo su: Ha senso usare Either in dart? Quali vantaggi porta e difetti ha rispetto a tryCatch? E per ora non mi hai per niente convinto :=)
Sarebbero bello che anche altre persone partecipassero alla discussione
Il problema secondario ma non risolto perchè è un BrakingChange è in produzione, questo approccio alle use case basato sugli Either porta ad alcuni problemi:
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:
if ( is )
per ogni failureprovider
ma richiede una scrittura prolissa per il recupero del valorefinal res = context.read<Either<Failure, Success>>()
e poco usabile/usato lato presentation ThrowCatch:on
provider
non è possibile. E' veramente necessario? No, dovresti passare sempre per ilbloc
appena la logica diventa complicata la quale necessita l'utilizzo diprovider
Sono mostrati solo esempi sicuri da scrivere, che non possono presentare errori di tipo Con Either: Esempio 1:
Esempio 2:
Con throw e catch Esempio 3:
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:
Con throw e catch: Esempio 2:
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