algolia / algolia/algoliasearch-client-javascript

Throw real Errors instead of objects

Aperta
#1,444 3 commenti 0 reazioni 0 assegnatari Vedi su GitHub
enhancement
Lingua principale
TypeScript
Stelle
1.4k
Fork
226
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

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.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Direzione di ricerca

Inizia da packages/client-account/src/methods/accountCopyIndex.ts e packages/client-account/src/errors/createDestinationIndiceExistsError.ts, poi esamina gli altri helper degli errori a cui fa riferimento l’issue. Decidi una rappresentazione coerente di Error e verifica che i fallimenti lanciati mantengano i loro nomi, messaggi e stack trace reali per consumer come Sentry.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
typescript
Ambito
api
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.