clase crashlytics #3
Conversation
dam788
requested review from
colomfernando,
christian97dd,
pablodamianberdun,
Maukr,
WilliamSaya-lvl30,
GonzaFran and
pablonortiz
lib/index.js
Outdated
| * const error = throw Error('params are required'); | ||
| * recordError(error, 'Required params') | ||
| */ | ||
| async recordError(error, jsErrorName) { |
There was a problem hiding this comment.
Según lo conversado, se tiene que agregar un 3er parametro opcional que sean los atributos extra que queremos logear junto a este error. En caso de pasarlos se debe validar que lo recibido sea un objeto valido
lib/index.js
Outdated
| if (!error) throw new Error('error is required'); | ||
| if (!!jsErrorName && typeof jsErrorName !== 'string') | ||
| throw new Error('incorrect type of jsErrorName'); | ||
|
|
There was a problem hiding this comment.
Según lo conversado, se tiene que agregar la función setAttribute, que se utilizará para darle mayor contexto al error y que, inicialmente, debe tener que enviar la información acumulada en userData. Sumado a esta información, y en caso de que sean validos, se debe agregar la data del objeto recibido como 3er parametro opcional.
Recordar que se debe priorizar los valores de userData por sobre los de los atributos opcionales
lib/index.js
Outdated
| * crash.log('this is a pda error', {info: {name: 'Pedro', email: 'pedro@email.com', age: '38'}}) | ||
| */ | ||
| log(message, attributes = {}) { | ||
| const attributesData = {...this.userData, ...attributes}; |
There was a problem hiding this comment.
Acá invertir el orden en que se destructuran las propiedades para darle prioridad a los valores de userData.
const attributesData = { ...attributes, ...this.userData}
There was a problem hiding this comment.
@dam788 Estuve viendo el package de analytics también un poco, y veo que la estructura de los archivos es un toque diferente.
Por ejemplo, en analytics tienen la clase en un file 'analytics.js' y desde el index lo exportan.
Lo mismo con como se manejan las utils.
Nose si es parte de este desarrollo pero estaría bueno unificar esto
lib/index.js
Outdated
| const setLogAttributes = async (attributes, key) => { | ||
| try { | ||
| attributes[key]?.constructor === Object | ||
| ? await setAttributes(attributes[key]) |
There was a problem hiding this comment.
Hola @dam788 parece que setAttributes y setAttribute quedaron sin el import
There was a problem hiding this comment.
Perfecto, ya estan las correcciones.
LINK DE TICKET:
https://janiscommerce.atlassian.net/browse/APPSRN-241
DESCRIPCIÓN DEL REQUERIMIENTO:
Contexto:
Al recibir un error en la consola de firebase crashlytics no hay suficiente información del usuario, lo que a veces hace mas difícil reconocer la causa del error o bien cada vez que se utiliza el método tenemos que repetir data innecesariamente
Necesidad:
Crear una clase que contenga todo lo mismo de antes, pero que pueda recibir por el constructor data para ser inicializada una sola vez, y aunque se utilice el método log y no le pases data, siempre tenga la data del usuario de forma predeterminada.
DESCRIPCIÓN DE LA SOLUCIÓN:
Se creó una clase Crashlytics pensando en la ventaja de inicializar la clase una sola vez y poder utilizarla en todo el proyecto. Desde el constructor va a setear userData que espera un objeto (pensando en getUserInfo).
A la clase Crashlytics se le agregó los métodos ya existentes:
Se agregaron los test en un solo archivo.
Actualización pedida por Gonzalo:
CÓMO SE PUEDE PROBAR?
yalc push && yalc publishyalc add @janiscommerce/app-crashlyticsynpm iNOTA: puede armarse una rama de pruebas como LINK, donde ya se probó el funcionamiento del desarrollo. Si quiere generar pruebas propias debe generar un google-services.json nuevo y no lleva mucho tiempo LINK
SCREENSHOTS:
DATOS EXTRA A TENER EN CUENTA: