algolia / algolia/algoliasearch-client-javascript

Throw real Errors instead of objects

Abierto
#1,444 3 comentarios 0 reacciones 0 asignados Ver en GitHub
enhancement
Lenguaje dominante
TypeScript
Estrellas
1.4k
Forks
226
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

This package has a non standard behavior of throwing objects instead of real JS Errors. As it works approximately in the same way in most cases and these objects are similar to Error (at least, TS is ok with that), there are some edge cases where it messes everything up.

My specific use case is with Sentry, which is not able to handle properly these "errors". Instead of having a proper stack trace, I have the Sentry error [Non-Error exception captured](https://sentry.zendesk.com/hc/en-us/articles/360057389753-Why-am-I-seeing-events-with-Non-Error-exception-or-promise-rejection-captured-with-keys-using-the-JavaScript-SDK-) and all the informations about the real error are lost.

We do this kind of thing all over the package:

[accountCopyIndex.ts](https://github.com/algolia/algoliasearch-client-javascript/blob/3aec216a9704f818b4faaa8b97d988ec2794f9da/packages/client-account/src/methods/accountCopyIndex.ts#L33)

```JS
throw createDestinationIndiceExistsError()
```

[createDestinationIndiceExistsError.ts](https://github.com/algolia/algoliasearch-client-javascript/blob/master/packages/client-account/src/errors/createDestinationIndiceExistsError.ts)

```JS
export function createDestinationIndiceExistsError(): Error {
return {
name: 'DestinationIndiceAlreadyExistsError',
message: 'Destination indice already exists.',
};
}
```

I propose instead to either throw a basic Error with the message...

```JS
export function createDestinationIndiceExistsError(): Error {
return new Error('DestinationIndiceAlreadyExistsError');
}
```

Or even to create custom error objects

```JS
class AlgoliaDestinationIndiceAlreadyExistsError extends Error {
constructor(message) {
super(message);
this.name = 'AlgoliaDestinationIndiceAlreadyExistsError';
}
}

export function createDestinationIndiceExistsError(): Error {
return new AlgoliaDestinationIndiceAlreadyExistsError('Destination indice already exists.');
}
```

I even think that this helper methods are not necessary and that custom errors could be thrown directly. For instance, the custom message is useless in this case.

I can open a PR with a proof of concept if you think it's a good idea. I would also be glad to hear your ideas about the right implementation.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Línea de trabajo

Comienza con packages/client-account/src/methods/accountCopyIndex.ts y packages/client-account/src/errors/createDestinationIndiceExistsError.ts; después, inspecciona los otros helpers de errores referenciados por el issue. Decide cuál es la representación coherente de Error y verifica que los fallos lanzados conserven sus nombres, mensajes y stack traces reales para consumidores como Sentry.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
typescript
Área
api
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
35/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.