falcor icon indicating copy to clipboard operation
falcor copied to clipboard

Inconsistencies in Error reporting between recycleJSON true and false

Open jameslaneconkling opened this issue 8 years ago • 2 comments

const model = new Model({
  source: {
    get: () => Observable.timer(100)
      .concat(Observable.throw({
        status: 500
      }))
  },
  recycleJSON
})


model.get(['items', 0, 'title'])  
  .subscribe({
    error(err) {
      console.log('Error', err)
    }
  });

If recycleJSON is true, logs:

// > { $type: 'error', value: { status: 500 } }

If recycleJSON is false, logs:

// > [{ "path": ["items", 0, "title"], "value": { "status":500 } }]

jameslaneconkling avatar Sep 01 '17 17:09 jameslaneconkling

actually, looks like recyleJSON: true just sets treatErrorsAsValues, which is fine by me. However, another inconsistency arises:

const model = new Model({
  source: {
    get: () => Observable.timer(100)
      .concat(Observable.throw({
        status: 500
      }))
  },
  recycleJSON
})
  .treatErrorsAsValues();


model.get(['items', 0, 'title'])  
  .subscribe({
    next(data) {
      console.log('Data', data);
    }
    error(err) {
      console.log('Error', err);
    }
  });

If recycleJSON is true, logs:

// > Error: { $type: 'error', value: { status: 500 } }

If recycleJSON is false, logs:

// > Data: { "items": {"0": {"title": { "status": 500 } } } }
// > Error: { $type: 'error', value: { status: 500 } }

And if run against Netflix's model, logs:

// > Data: { "items": {"0": {"title": { "status": 500 } } } }

I'm not 100% sure what the desirable behavior is. Probably makes sense for behavior to be consistent between recycleJSON true/false. Not sure whether or not it also makes sense to track behavior in Netflix's model as well...

jameslaneconkling avatar Sep 01 '17 17:09 jameslaneconkling

@jameslaneconkling you're right, the recycleJSON version should be onNext'ing value at minimum. Will look into it now.

trxcllnt avatar Sep 04 '17 20:09 trxcllnt