El guardián que nunca estuvo

El motor de Greenhouse tiene un puñado de funciones que mueven un proyecto a través de su ciclo de vida: promover una idea, registrar un toque, avanzar una etapa, guardarla en el vault, publicarla. Cada una de ellas lee el estado actual del elemento antes de decidir qué hacer. Ninguna verificaba si ese estado era en realidad un punto de partida sensato.

Es una cosa rara de notar en código que llevaba semanas en producción. Funcionaba porque la interfaz solo ofrecía estas acciones desde el lugar correcto, un botón de “Vault” solo aparece en un elemento que se puede guardar en el vault. Pero las funciones del motor en sí no lo exigían. Llamar a vault_item dos veces sobre el mismo elemento (un doble clic, un reintento, un futuro llamador que no sea la interfaz actual) volvería a estampar felizmente el timestamp de vaulting, moviendo en silencio la marca de cuánto tiempo llevaba algo inactivo. Llamar a promote_idea sobre un proyecto que ya lleva tres etapas de avance lo reiniciaría de vuelta al principio.

Agregar las verificaciones fue la parte fácil

Cuatro funciones, cuatro guardas de una línea, todas usando un tipo de error que ya existía. Rechazar el estado inicial incorrecto, retornar temprano, listo. Escribí la guarda, escribí una prueba que guarda algo en el vault dos veces y verifica que el timestamp no se moviera, y pasé a la siguiente función.

La interesante fue la quinta función, la que el issue ni siquiera mencionaba.

La función que llamaba a otra función dejó de poder hacerlo

Avanzar un proyecto a su siguiente etapa hace dos cosas: registra un toque (se trabajó en esto, por eso se mueve) y mueve la carpeta al directorio de la nueva etapa. Hasta ahora esos eran dos commits de base de datos separados: registrar el toque y, en un segundo paso, mover la etapa. Si el segundo paso fallaba, el primero ya había pasado. El proyecto se hundiría al fondo de la lista de trabajo como si se hubiera trabajado en él recién, sin realmente haber avanzado a ningún lado.

Arreglar eso significaba combinar las dos escrituras en una sola transacción. Bastante simple, excepto que el paso de “registrar un toque” no era una escritura de base de datos propia dentro de esta función, era una llamada a la función dedicada de registro de toques, que abre y confirma su propia transacción internamente. No se puede anidar una transacción de SQLite dentro de otra en la misma conexión. Así que en el momento en que escribí una sola transacción envolviendo las dos operaciones, ya no podía llamar a esa otra función. Tuve que meterme y copiar sus dos sentencias de base de datos individuales directamente dentro del nuevo bloque combinado.

Ahí fue donde se puso interesante: la función de registro de toques era una de las cuatro a las que acababa de agregarle una guarda. Llamarla solía ser la razón por la que avanzar una etapa estaba a salvo de un mal estado inicial, si el elemento no estaba en el estado correcto, la propia llamada al toque se negaría y detendría todo. Una vez que dejé de llamarla y en su lugar puse sus sentencias en línea, esa protección no vino incluida. Nunca quedó escrito en ningún lado como “esta función es segura porque delega en aquella otra”. Simplemente era cierto, hasta que un cambio de cañería por un motivo sin relación hizo que dejara de serlo, en silencio.

La solución fue escribir la misma guarda una segunda vez, directamente en la función que ya no podía tomarla prestada de su vecina. Lo cual significó que esta función necesitaba una verificación que el reporte original del bug nunca pidió, porque el reporte se escribió antes de que nadie hubiera rastreado lo que en realidad iba a requerir combinar estas dos escrituras.

Probar un fallo sin autoengañarse

Parte de este trabajo significaba probar que un movimiento de carpeta fallido deja todo intacto, y que una escritura de archivo fallida después de una inserción exitosa en la base de datos no deja una fila huérfana suelta. Ambos casos necesitan una forma de hacer fallar de manera confiable una operación de sistema de archivos dentro de una prueba.

El truco obvio es poner una carpeta en solo lectura e intentar escribir dentro. Lo usé antes y funciona bien, hasta que deja de funcionar: si la prueba resulta correr como root, los permisos dejan de importar, la escritura tiene éxito de todas formas, y la prueba pasa por una razón completamente equivocada. Se ve en verde. No está probando nada.

El arreglo que en realidad se sostiene sin importar quién corra la prueba: poner el tipo equivocado de cosa en la ruta, en vez de los permisos equivocados. ¿Se necesita que falle una llamada de creación de directorio? Basta con poner ahí primero un archivo plano, no hay forma de convertir un archivo en un directorio, sin importar quién lo intente. ¿Se necesita que falle una escritura de archivo? Poner ahí un directorio en su lugar. Root no tiene ninguna excepción especial frente a “eso no es un directorio.”

En el proceso, me equivoqué una vez de una forma que vale la pena mencionar porque es una trampa fácil: una operación dentro del código bajo prueba mueve una carpeta entera a su lugar con un rename, en vez de escribir directamente dentro. Puse mi archivo bloqueador justo en el destino, y la prueba falló, pero por la razón equivocada. Renombrar una carpeta sobre un destino existente y no vacío falla por sí solo, antes de que la ruta de código que en realidad quería probar llegue a correr. La solución fue poner el archivo bloqueador dentro de la carpeta que se estaba moviendo, para que viajara junto con el rename y solo causara problemas una vez que aterrizara donde en realidad quería interrumpir algo.

Lo que se me quedó grabado

La forma recurrente acá no es “agregar validación”, es “la corrección de una función a veces depende de cómo se la llama, no solo de qué hace”. Combinar las escrituras de dos funciones en una sola transacción es un cambio de cañería, no parece que debiera tocar el comportamiento. Pero si una de esas funciones estaba actuando en silencio como guardián de la otra, quitar la llamada quita la guarda, y nada en el sistema de tipos avisa que eso pasó. La única forma en que lo detecté fue preguntando, función por función, “qué pasaría ahora mismo si llamo a esta desde un estado que no esperaba”, y esa es una pregunta que vale la pena volver a hacer cada vez que un refactor cambia quién llama a quién, no solo una vez, cuando las guardas se escriben por primera vez.