Manejo de múltiples capturas en la cadena de promesa

125

Todavía soy bastante nuevo en las promesas y estoy usando bluebird actualmente, sin embargo, tengo un escenario en el que no estoy muy seguro de cómo tratarlo mejor.

Entonces, por ejemplo, tengo una cadena de promesa dentro de una aplicación express como esta:

repository.Query(getAccountByIdQuery)
        .catch(function(error){
            res.status(404).send({ error: "No account found with this Id" });
        })
        .then(convertDocumentToModel)
        .then(verifyOldPassword)
        .catch(function(error) {
            res.status(406).send({ OldPassword: error });
        })
        .then(changePassword)
        .then(function(){
            res.status(200).send();
        })
        .catch(function(error){
            console.log(error);
            res.status(500).send({ error: "Unable to change password" });
        });

Entonces el comportamiento que busco es:

  • Va a obtener cuenta por ID
  • Si hay un rechazo en este punto, bombardea y devuelve un error
  • Si no hay error, convierta el documento devuelto a un modelo
  • Verifique la contraseña con el documento de la base de datos
  • Si las contraseñas no coinciden, bombardea y devuelve un error diferente
  • Si no hay error, cambie las contraseñas
  • Luego devuelve el éxito
  • Si algo más salió mal, devuelve 500

Entonces, las capturas actuales no parecen detener el encadenamiento, y eso tiene sentido, por lo que me pregunto si hay una manera de forzar de alguna manera a la cadena a detenerse en un cierto punto en función de los errores, o si hay una mejor manera estructurar esto para obtener algún tipo de comportamiento de ramificación, como es el caso de if X do Y else Z.

Cualquier ayuda sería genial.

Grofit
fuente
¿Puedes volver a tirar o regresar temprano?
Pieter21

Respuestas:

126

Este comportamiento es exactamente como un lanzamiento sincrónico:

try{
    throw new Error();
} catch(e){
    // handle
} 
// this code will run, since you recovered from the error!

Esa es la mitad del punto de .catchpoder recuperarse de los errores. Puede ser conveniente volver a lanzar para indicar que el estado sigue siendo un error:

try{
    throw new Error();
} catch(e){
    // handle
    throw e; // or a wrapper over e so we know it wasn't handled
} 
// this code will not run

Sin embargo, esto solo no funcionará en su caso ya que el error será detectado por un controlador posterior. El problema real aquí es que los manejadores de errores generalizados "MANEJAR CUALQUIER COSA" son una mala práctica en general y están muy mal vistos en otros lenguajes de programación y ecosistemas. Por esta razón, Bluebird ofrece capturas tipificadas y predicadas.

La ventaja adicional es que su lógica de negocios no tiene (y no debería) tener que conocer el ciclo de solicitud / respuesta. No es responsabilidad de la consulta decidir qué estado HTTP y error obtiene el cliente y, a medida que su aplicación crezca, es posible que desee separar la lógica de negocios (cómo consultar su base de datos y cómo procesar sus datos) de lo que envía al cliente (qué código de estado http, qué texto y qué respuesta).

Así es como escribiría tu código.

Primero, .Querylanzaría un NoSuchAccountError, lo subclase de lo Promise.OperationalErrorque Bluebird ya proporciona. Si no está seguro de cómo subclasificar un error, avíseme.

Además, lo subclasificaría AuthenticationErrory luego haría algo como:

function changePassword(queryDataEtc){ 
    return repository.Query(getAccountByIdQuery)
                     .then(convertDocumentToModel)
                     .then(verifyOldPassword)
                     .then(changePassword);
}

Como puede ver, está muy limpio y puede leer el texto como un manual de instrucciones de lo que sucede en el proceso. También está separado de la solicitud / respuesta.

Ahora, lo llamaría desde el controlador de ruta como tal:

 changePassword(params)
 .catch(NoSuchAccountError, function(e){
     res.status(404).send({ error: "No account found with this Id" });
 }).catch(AuthenticationError, function(e){
     res.status(406).send({ OldPassword: error });
 }).error(function(e){ // catches any remaining operational errors
     res.status(500).send({ error: "Unable to change password" });
 }).catch(function(e){
     res.status(500).send({ error: "Unknown internal server error" });
 });

De esta manera, la lógica está en un solo lugar y la decisión de cómo manejar los errores al cliente está en un solo lugar y no se abarrotan entre sí.

Benjamin Gruenbaum
fuente
11
Es posible que desee agregar que la razón para tener un .catch(someSpecificError)controlador intermedio para algún error específico es si desea detectar un tipo específico de error (que es inofensivo), tratarlo y continuar el flujo que sigue. Por ejemplo, tengo un código de inicio que tiene una secuencia de cosas que hacer. Lo primero es leer el archivo de configuración del disco, pero si ese archivo de configuración falla, es un error correcto (el programa ha incorporado los valores predeterminados) para que pueda manejar ese error específico y continuar el resto del flujo. También puede haber una limpieza mejor para no irse hasta más tarde.
jfriend00
1
Pensé que "Esa es la mitad del punto de .catch: poder recuperarse de los errores" lo dejó claro, pero gracias por aclarar más, ese es un buen ejemplo.
Benjamin Gruenbaum
1
¿Qué pasa si no se usa bluebird? Las promesas sencillas de es6 solo tienen un mensaje de error de cadena que se pasa para capturar.
Relojero
3
@clocksmith con ES6 promete que estás atrapado atrapando todo y haciendo instanceofchceks manualmente tú mismo.
Benjamin Gruenbaum
1
Para aquellos que buscan una referencia para la subclase de objetos Error, lea bluebirdjs.com/docs/api/catch.html#filtered-catch . El artículo también reproduce más o menos la respuesta de captura múltiple dada aquí.
mummybot
47

.catchfunciona como la try-catchdeclaración, lo que significa que solo necesita una captura al final:

repository.Query(getAccountByIdQuery)
        .then(convertDocumentToModel)
        .then(verifyOldPassword)
        .then(changePassword)
        .then(function(){
            res.status(200).send();
        })
        .catch(function(error) {
            if (/*see if error is not found error*/) {
                res.status(404).send({ error: "No account found with this Id" });
            } else if (/*see if error is verification error*/) {
                res.status(406).send({ OldPassword: error });
            } else {
                console.log(error);
                res.status(500).send({ error: "Unable to change password" });
            }
        });
Esailija
fuente
1
Sí, sabía esto, pero no quería hacer una gran cadena de errores, y parecía más legible hacerlo cuando lo necesitaba. De ahí el truco al final, pero me gusta la idea de los errores mecanografiados, ya que es más descriptivo en cuanto a la intención.
Grofit
8
@Grofit por lo que vale: las capturas escritas en Bluebird fueron idea de Petka (Esailija) para empezar :) No es necesario convencerlo de que es un enfoque preferible aquí. Creo que no quería confundirte, ya que muchas personas en JS no son muy conscientes del concepto.
Benjamin Gruenbaum
17

Me pregunto si hay una manera de forzar de alguna manera a la cadena a detenerse en cierto punto en función de los errores

No. No puedes realmente "terminar" una cadena, a menos que arrojes una excepción que burbujee hasta su final. Vea la respuesta de Benjamin Gruenbaum sobre cómo hacer eso.

Una derivación de su patrón sería no distinguir los tipos de error, sino utilizar los errores que tienen statusCodey los bodycampos que pueden enviarse desde un único .catchcontrolador genérico . Dependiendo de la estructura de su aplicación, su solución podría ser más limpia.

o si hay una mejor manera de estructurar esto para obtener algún tipo de comportamiento de ramificación

Sí, puedes hacer ramificaciones con promesas . Sin embargo, esto significa abandonar la cadena y "volver" a anidar, tal como lo haría en una instrucción anidada if-else o try-catch:

repository.Query(getAccountByIdQuery)
.then(function(account) {
    return convertDocumentToModel(account)
    .then(verifyOldPassword)
    .then(function(verification) {
        return changePassword(verification)
        .then(function() {
            res.status(200).send();
        })
    }, function(verificationError) {
        res.status(406).send({ OldPassword: error });
    })
}, function(accountError){
    res.status(404).send({ error: "No account found with this Id" });
})
.catch(function(error){
    console.log(error);
    res.status(500).send({ error: "Unable to change password" });
});
Bergi
fuente
5

He estado haciendo de esta manera:

Dejas tu captura al final. Y solo arroje un error cuando ocurra a mitad de su cadena.

    repository.Query(getAccountByIdQuery)
    .then((resultOfQuery) => convertDocumentToModel(resultOfQuery)) //inside convertDocumentToModel() you check for empty and then throw new Error('no_account')
    .then((model) => verifyOldPassword(model)) //inside convertDocumentToModel() you check for empty and then throw new Error('no_account')        
    .then(changePassword)
    .then(function(){
        res.status(200).send();
    })
    .catch((error) => {
    if (error.name === 'no_account'){
        res.status(404).send({ error: "No account found with this Id" });

    } else  if (error.name === 'wrong_old_password'){
        res.status(406).send({ OldPassword: error });

    } else {
         res.status(500).send({ error: "Unable to change password" });

    }
});

Sus otras funciones probablemente se verían así:

function convertDocumentToModel(resultOfQuery) {
    if (!resultOfQuery){
        throw new Error('no_account');
    } else {
    return new Promise(function(resolve) {
        //do stuff then resolve
        resolve(model);
    }                       
}
Leo Leao
fuente
4

Probablemente un poco tarde para la fiesta, pero es posible anidar .catchcomo se muestra aquí:

Red de desarrolladores de Mozilla: uso de promesas

Editar: envié esto porque proporciona la funcionalidad solicitada en general. Sin embargo, no lo hace en este caso particular. Porque como ya explicaron en detalle otros, .catchse supone que debe recuperar el error. No se puede, por ejemplo, enviar una respuesta al cliente en múltiples .catch devoluciones de llamada porque un .catchsin explícitas return resuelve TI con undefineden ese caso, causando procedimiento .thende desencadenar pesar de que su cadena no está realmente resuelto, que puede causar una siguiente .catchal gatillo y el envío otra respuesta al cliente, causando un error y probablemente arrojando un UnhandledPromiseRejectionrumbo. Espero que esta oración enrevesada tenga algún sentido para ti.

denkquer
fuente
1
@AntonMenshov Tienes razón. Amplié mi respuesta, explicando por qué su comportamiento deseado todavía no es posible con la anidación
denkquer
2

En lugar de .then().catch()...que puedas hacer .then(resolveFunc, rejectFunc). Esta cadena de promesa sería mejor si manejas las cosas en el camino. Así es como lo reescribiría:

repository.Query(getAccountByIdQuery)
    .then(
        convertDocumentToModel,
        () => {
            res.status(404).send({ error: "No account found with this Id" });
            return Promise.reject(null)
        }
    )
    .then(
        verifyOldPassword,
        () => Promise.reject(null)
    )
    .then(
        changePassword,
        (error) => {
            if (error != null) {
                res.status(406).send({ OldPassword: error });
            }
            return Promise.Promise.reject(null);
        }
    )
    .then(
        _ => res.status(200).send(),
        error => {
            if (error != null) {
                console.error(error);
                res.status(500).send({ error: "Unable to change password" });
            }
        }
    );

Nota: El if (error != null)es un poco de un truco para interactuar con el error más reciente.

mvndaai
fuente
1

Creo que la respuesta de Benjamin Gruenbaum anterior es la mejor solución para una secuencia lógica compleja, pero esta es mi alternativa para situaciones más simples. Solo uso una errorEncounteredbandera junto con return Promise.reject()para omitir cualquier posterior theno catchdeclaraciones. Entonces se vería así:

let errorEncountered = false;
someCall({
  /* do stuff */
})
.catch({
  /* handle error from someCall*/
  errorEncountered = true;
  return Promise.reject();
})
.then({
  /* do other stuff */
  /* this is skipped if the preceding catch was triggered, due to Promise.reject */
})
.catch({
  if (errorEncountered) {
    return;
  }
  /* handle error from preceding then, if it was executed */
  /* if the preceding catch was executed, this is skipped due to the errorEncountered flag */
});

Si tiene más de dos pares / catch, probablemente debería usar la solución de Benjamin Gruenbaum. Pero esto funciona para una configuración simple.

Tenga en cuenta que la final catchsolo tiene en return;lugar de return Promise.reject();, porque no hay subsecuentes thenque debamos omitir, y contaría como un rechazo de Promesa no controlado, que a Node no le gusta. Como está escrito arriba, la final catchdevolverá una promesa resuelta pacíficamente.

nombre_usuario_temporal
fuente