Upgrade bluebird to 3.x

[nkt]

No description provided.

[neumino]

Thanks, but this breaks the API. Thinky should return operational errors when they are, so tests shouldn't be updated.

[nkt]

So maybe thinky should use own Promise library like this:

var Promise = require("bluebird/js/main/promise")();
Promise.prototype.error = Promise.prototype.catch;

I can update PR.

[neumino]

Nah that wouldn't work because people can still throw in the thenHandler and it would be passed to error not catch

[nfantone]

This is basically replacing error for catch. How does this break the API?

[neumino]

They can throw error inside the then handler and it will be catch by error before being bubbled up to catch

[nfantone]

@neumino Yes, I read your first comment when you were explaining that. But I don't think I follow. This are all the breaking changes in Bluebird 3.0. What exactly in that list would "break" thinky's API? Both .error and .catch are part of 3.0. You could leave all .error calls exactly like they are now, if you'd like.

Moreover, assuming the API changes and you would like to cut a release, you have semver to back you up.

I'm saying all these because it looks like thinky is falling behind on updates. Dependency versions are locked. It's not even using latest rethinkdbdash (heck, mocha 1.21.5 dates from Oct 11, 2014!)

[neumino]

What changes is that Promise.reject doesn't automatically create an operational error. So just need to manually create the operational error.
Something similar to what I did for rethinkdbdash in neumino/rethinkdbdash@03b6ed1

About the dependencies, rethinkdbdash is update to date. If you install thinky 2.3, you get rethinkdbdash 2.3.x (though it looks like npm is not smart enough and falls to 2.2.8 now by default).

mocha is old and I'm not aware of a reason to update but I'll merge a pull request if the tests still work.

[nfantone]

Thanks for the explanation! If you want, I could try and send a new PR for this.

(though it looks like npm is not smart enough and falls to 2.2.8 now by default).

That's what I'm getting, yes.

mocha is old and I'm not aware of a reason to update

No reason, really. I was just trying to make a point about updates.

[digital-flowers]

isn't this can be solved just by upgrading thinky to version 3.0
i think upgrading bluebird to v3 is very important, it is very annoying that i have to wrap every thinky promise with new promise to make it compatible with the rest of the system

[digital-flowers]

we can create a special branch for thinky version 2 for LTS and just upgrading bluebird to version 3

[nfantone]

I second that. I'm offering myself for a PR if needed.

[neumino]

Maintaining two branches is just more work. If someone send a PR that update thinky to bluebird 3 while being backward compatible, I'll merge it.

As far as I know, it's just about making all the errors called with reject operational.

[digital-flowers]

ok after taking a look at the code i think we need to fix the following:

• upgrade rethinkdbdash to version 2.3.25
• change every error handled by Promise.error to be instance of OperationalError
• change every Promise.nodeify to Promise.asCallback
• i noticed some promises without return this will lead to `Warning: a promise was created in a handler but none were returned from it'
• backward compatibility as suggested by the owner (@neumino)

[neumino]

No, you shouldn't change Promise.error to Promise.catch, you just want to make the error operational.
http://bluebirdjs.com/docs/api/operationalerror.html

[digital-flowers]

my bad you are totally right, sorry usually i always used to do normal rejection :P

[nfantone]

@digital-flowers rethinkdbdash@2.3.5 is the version that you get when you install thinky.

├─┬ rethinkdbdash@2.3.25
│ └── bluebird@3.4.6
└── validator@3.22.2

[digital-flowers]

ooh so rethinkdbdash already using bluebird 3

[nfantone]

I started working on a PR for this.

@neumino Could you elaborate on why you'd like to change errors to OperationalError? Is it to make it "backwards compatible" and let people use .error to manage everything? It seems hacky. See this issue. The change was made to make Bluebird compliant with the native Promise spec.

If that is case, then semver if what you need. Not masking your errors. Just release 3.0.0 with bluebird@3 support.

[nfantone]

To clarify, what I believe you want is this:

const Promise = require('bluebird');

new Promise(function(resolve, reject) {
  var e = new Error('boom');
  e.isOperational = true; // Basically, the same as `new OperationalError()`
  return reject(e);
}).error((err) => {
  console.log('Got an OperationalError');
});

But then, the Promise spec states that there is no .error method on Promise. You really should be using .catch, instead.

[nfantone]

☝️ There it is, @neumino. Let's hope you reconsider your position on errors.

If not, well ¯_(ツ)_/¯.

[neumino]

The changes were in bluebird 3 were to make bluebird compatible with the spec.
It doesn't mean that there has to be a breaking change in thinky.

The breaking change is that reject doesn't automatically create an operational error.
The definition of error is here[1] and I believe it's the same between bluebird 2 and 3.
Just upgrading to bluebird 3 means that anyone currently catching rejects with error have to update their code to use catch.

[1]http://bluebirdjs.com/docs/api/error.html

[nfantone]

The changes were in bluebird 3 were to make bluebird compatible with the spec.

Correct. That's what I explained my previous comment.

It doesn't mean that there has to be a breaking change in thinky.

It does if you want to use Bluebird 3.

Just upgrading to bluebird 3 means that anyone currently catching rejects with error have to update their code to use catch.

Yes, we got that. But upgrading is not enforced to users. It's entirely up to them. So, again, if you do a MAJOR bump in thinky everything's good and dandy.

I completely understand if you don't want to do that, but your proposed alternative seems like a bad design decision in the long term.

[neumino]

It does if you want to use Bluebird 3.

No it doesn't. Rethinkdbdash was migrated to bluebird 3 without introducing breaking change.

Bumping to a major number doesn't remove the pain for people to update their code if they want to update thinky.