JSON.parse is not wrapped with try/catch about node-fetch

[niftylettuce]

I'm trying to fork/patch right now... will submit a PR if I figure it out

[niftylettuce]

Before:

/**
 * Decode response as json
 *
 * @return  Promise
 */
Response.prototype.json = function() {

    return this._decode().then(function(text) {
        return JSON.parse(text);
    });

}

After:

/**
 * Decode response as json
 *
 * @return  Promise
 */
Response.prototype.json = function() {

    return this._decode().then(function(text) {
    try {
      var body = text && JSON.parse(text);
    } catch (e) {
      var err = e;
    } finally {
      if (err)
        return Response.Promise.reject(err);
      return body;
    }
    });

}

[TimothyGu]

With a proper Promises/A+ (or ES2015, which is based on the Promises/A+ spec) implementation (like the one used by this module), this is not needed. In fact, this pattern is exactly the kind Promises are trying to prevent.

Do you have a specific test case where the method is not working properly?

[TimothyGu]

Specs:

• Promises/A+ 2.2.7.2

• ECMAScript 2015:

  25.4.1.3.2 Promise Resolve Functions
  8. Let then be Get(resolution, "then").
  9. If then is an abrupt completion, then
    a. Return RejectPromise(promise, then.[[value]]).

  25.4.2.1 PromiseReactionJob ( reaction, argument )
  6. Else, let handlerResult be Call(handler, undefined, «argument»).
  7. If handlerResult is an abrupt completion, then
    a. Let status be Call(promiseCapability.[[Reject]], undefined, «handlerResult.[[value]]»).
    b. NextJob Completion(status).

[niftylettuce]

doesn't that spec say if abrupt completion then use reject promise? isn't
throwing an error an abrupt completion?
On Nov 26, 2015 6:55 PM, "Timothy Gu" [email protected] wrote:

Specs:

• Promises/A+ 2.2.7.2 https://promisesaplus.com/#point-42

  ECMAScript 2015:

  25.4.1.3.2
  http://www.ecma-international.org/ecma-262/6.0/#sec-promise-resolve-functions
  Promise Resolve Functions

• Let then be Get(resolution, "then").

• If then is an abrupt completion, then
  a. Return RejectPromise(promise, then.[[value]]).

  25.4.2.1
  http://www.ecma-international.org/ecma-262/6.0/#sec-promisereactionjob
  PromiseReactionJob ( reaction, argument )

• Else, let handlerResult be Call(handler, undefined, «
  argument»).

• If handlerResult is an abrupt completion, then
  a. Let status be Call(promiseCapability.[[Reject]], undefined,
  «handlerResult.[[value]]»).
  b. NextJob Completion(status).

—
Reply to this email directly or view it on GitHub
#59 (comment).

[TimothyGu]

You are correct. But isn't "rejected promise" exactly what you are trying to do? At least in your implementation you are returning a rejected promise in the onFulfilled callback.

Or am I missing something?

[bitinn]

I don't see why we should catch JSON parser error manually, you should be able to handle it in catch clause as you handle other type of network errors. Maybe an example might help to explain your problem.

[niftylettuce]

See https://github.com/niftylettuce/frisbee/blob/master/src/frisbee.js#L108-L114 for my implementation