KyleAMathews / superagent-bluebird-promise

Add promise support to superagent using Bluebird
MIT License
182 stars 37 forks source link

Warning: a promise was rejected with a non-error: [object Object] #68

Closed chevin99 closed 7 years ago

chevin99 commented 8 years ago

I keep seeing this warning

Warning: a promise was rejected with a non-error: [object Object]

in the console when I make a request like below that results in an error (like a 500 status for example). The request works fine and I can catch the error no problem, but I'd like to get that warning to go away in the console. Any idea how?

let request = require('superagent-bluebird-promise')

request.post(url, data)
  .promise()
  .then((res) => {
    console.log(res);
  })
  .catch((e) => {
    console.log(e);
  })
chevin99 commented 8 years ago

According to this comment on bluebird issue 990, it seems that the warning happens when the error doesn't have all of the following: .name, .message, .stack.

When I change this in the source

if (Object.defineProperty) {
    Object.defineProperty(this, 'stack', {
      get: function() {
        if (this.originalError) {
          return stack + '\nCaused by:  ' + this.originalError.stack;
        }

        return stack;
      }
    });
  }

to this

this.stack = this.originalError
    ? stack + '\nCaused by:  ' + this.originalError.stack
    : stack;

the warning goes away. It seems that the warning happens because bluebird doesn't recognize the existence of .stack when defined using that Object.defineProperty method.

neekey commented 8 years ago

same here : (

oponder commented 7 years ago

Yeah I'm seeing a promise was rejected with a non-error in my console a lot too, and I'm one of those guys that likes to have a clean console as much as possible :)

Do you guys consider this something that should be fixed in Bluebird? or can we get a bit less fancy with the definition of the stack property on SuperagentPromiseError like @chevin99 suggested. I'd be happy to make a PR.

Here is the definition of SuperagentPromiseError: https://github.com/KyleAMathews/superagent-bluebird-promise/blob/master/index.js#L25

Anyone know what benefit the Object.defineProperty way gives us?

johanneslumpe commented 7 years ago

@oponder It's even worse that those warnings show up in production. @KyleAMathews can you chime in here?

KyleAMathews commented 7 years ago

I'm not using this lib in production right now so would be happy to make any of you interested collaborators so you can make the needed changes and do a new release.

johanneslumpe commented 7 years ago

@KyleAMathews sure, would love to do that over the weekend!

chevin99 commented 7 years ago

@johanneslumpe, any progress? Thanks for the help on this.

johanneslumpe commented 7 years ago

@chevin99 I didn't get to this yet :/

Gekkio commented 7 years ago

Making the stack property settable and configurable are the minimal steps needed to make Bluebird detect SuperagentBluebirdError objects as errors while keeping the Object.defineProperty call.

I've got a fix here but didn't yet submit a pull request as I'm not 100% sure it's the right fix. The warning is gone, but some functionality might behave differently.

evegreen commented 7 years ago

@Gekkio , your fix works perfectly, can u make PR please ?