search

Netflix / falcor

A JavaScript library for efficient data fetching
http://netflix.github.io/falcor
Apache License 2.0
10.48k stars 446 forks source link

Handling fromPath errors in SetResponse constructor #989

Closed chourihan closed 3 years ago

chourihan commented 3 years ago

When creating a new SetResponse instance, the constructor may make calls to pathSyntax.fromPath. If the presumed path is invalid, the parser may throw an exception which since done in the constructor is uncaught. This catches those exception and emits an error on _subscribe.

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.1%) to 92.836% when pulling dc0707030e2d04fffb5e38b5298cf9eec489a865 on catchErrorsInSetResponse into df45d9a98a1b2143291cb111fe26e079750ca828 on master.

jcranendonk commented 3 years ago

Good catch, thanks for the PR. Could you add a unit test for this scenario? Does this issue exist in GetResponse as well?

jcranendonk commented 3 years ago

On looking into SetResponse, it appears that input validation should be handled in lib/support/validateInput.js. Can you verify whether the faulty input is handled correctly in that function, and if not, fix it in that location?

chourihan commented 3 years ago

On looking into SetResponse, it appears that input validation should be handled in lib/support/validateInput.js. Can you verify whether the faulty input is handled correctly in that function, and if not, fix it in that location?

@jcranendonk it does look like this is invoked before the SetResponse constructor and is the more appropriate place to catch the error.

chourihan commented 3 years ago

@jcranendonk moved the error handling to lib/support/validateInput.js and added a couple of tests.