7

I am looking to standardize the use of Q promises in my team's codebase. Are there any good jscs extensions (or other linters) to help enforce style when it comes to promises?

We would like our promises to follow this form:

promise()
  .then()
  .catch()
  .done();

And would like a linter to catch any .then() in our code that is missing a .catch()

Advice for other stylistic tips when it comes to promises is welcome too.

Jeff
  • 2,425
  • 1
  • 18
  • 43
  • 4
    Most `then`s in any code should not be followed by `catch`, as you `return` promises from your functions. You'd use this pattern (if at all) only at the end of a promise chain, and I guess that is hard to track for a simple stylechecker. – Bergi Apr 21 '15 at 23:42
  • 2
    Elaborating on what Bergi said - `.then()` having to have a `.catch` and `.done` is like making every function wrapped in a `try/catch` or making every function that doesn't return a value explicitly say so. In my opinion if you want better debugging try bluebird promises instead of Q, they drop the requirement for `.done` in order to show exceptions and they also perform much better. **Basically, this is Qs fault for making you do this and you can avoid it with a modern library**. – Benjamin Gruenbaum Apr 22 '15 at 10:49
  • 2
    Also, this is off topic (library requests) but https://github.com/spion/thenlint – Benjamin Gruenbaum Apr 22 '15 at 10:50
  • @BenjaminGruenbaum thank you for the suggestions! The reason for enforcing `.catch` for every `.then` call was that we were having exceptions being swallowed with just `.then`, and catch seemed to be the way to fix it. For example, in a function that returns a deferred.promise: `function() { var def = Q.defer(); promise.then(function(result){ // do something to modify result def.resolve(result); }) .catch(function(err) { def.reject(err); }) return def.promise(); }` Would this be better handled with `.done` or something in BlueBird? – Jeff Apr 22 '15 at 16:50
  • 1
    @Jeff this is just because you're using an _ancient_ library, native promises (in io.js), RSVP promises, when promises and bluebird promise all give you tools to deal with this problem. Bluebird is the best of the bunch in how it handles it (it's also 100 times faster than Q, has better stack traces and a richer API) – Benjamin Gruenbaum Apr 22 '15 at 17:17

1 Answers1

2

@Jeff that approach looks as total overkill. Neither of this functions must be followed with any. Each of them has different purpose:

  • Use then(mapSuccess, mapFail) when you want to process resolved value and you need a result promise that will resolve with value returned by callback.
    Technically it's a mapping of value into other value which will be resolved by other promise. You may think of it similarly as of array's map with which you map input array into other which is result of some transformation function.
  • catch(mapFail) is purely alias for then(null, mapFail), So just use it when you want to then but you have no need to pass mapSuccess callback.
  • done(onSuccess, onFail) use simply, when all you want to do is to process resolved value (no need for mapping to other promise). done will also assure that all eventual errors are naturally exposed (then and catch as they're mappers, swallow errors into promise results).

I can imagine only one rule, which can be added for linter (and it's assuming that you use library which doesn't log swallowed exceptions). It's to warn about then() or catch() usages when their results are being ignored (they should be followed by done(..) or passed to other entity for processing).

Mariusz Nowak
  • 32,050
  • 5
  • 35
  • 37
  • we had decided to use `.then` `.catch` because we find it more readable than `then(onSuccess, onFail)`. But your done comment is interesting, as we wanted to implement this linting mainly to avoid swallowed errors. In a situation such as my comment in response to Benjamin on the original question, would `.done` be more appropriate than `.then` if we would like to avoid swallowed errors? – Jeff Apr 22 '15 at 17:00
  • I put argument names to better describe role of each method, and show that `.catch` is not really different from `.then`. I've additionally updated them now, to mark distinction between `.then` and `.done` :) – Mariusz Nowak Apr 23 '15 at 06:22
  • 2
    @Jeff: but those notations [are not equivalent](http://stackoverflow.com/a/24663315/1048572)?! – Bergi Apr 23 '15 at 08:35
  • Indeed, `.then(onSuccess,onError)` is not the same than `.then(onSuccess).catch(onError)`. The former does not catch error happening inside the *onSuccess* method. 98% of time the former is an anti-pattern. – cronvel Nov 17 '17 at 13:52