Uncatchable errors when there are invalid arguments

[enneid]

Try this code:

 fse = require('fs-extra')
try{
fse.move(null, null, (err, data) => {
console.log("Errr", err)})
}catch(err){
console.log("Err", err)
}

it will crash without chance to catch error.

[jprichardson]

So you're saying, you don't ever see "Errr"?

[enneid]

yes, try this code in pure node console (i tried it in node 5.6). In real application this will throw and ends brutally app.
starcktrace:

TypeError: dest path must be a string
at TypeError (native)
at Object.fs.link (fs.js:958:11)
at doRename (E:\Work\editor\node_modules\fs-extra\lib\move\index.js:67:10)
at E:\Work\editor\node_modules\fs-extra\lib\move\index.js:33:7
at E:\Work\editor\node_modules\fs-extra\lib\mkdirs\mkdirs.js:47:16
at FSReqWrap.oncomplete (fs.js:82:15)

[RyanZim]

This is at https://github.com/jprichardson/node-fs-extra/blob/master/lib/mkdirs/mkdirs.js#L32.

fs throws a TypeError if path is not a string. On one hand, I see @enneid's point, but this is a case where I think we should follow node.js' lead and throw an error.

Passing null is a programmer error, not a runtime error. If you have a unique case where this can be a runtime error, you will have to type-check your arguments before passing them to fs-extra.

CC: @jprichardson

[jprichardson]

fs throws a TypeError if path is not a string. On one hand, I see @enneid's point, but this is a case where I think we should follow node.js' lead and throw an error.

👍

[enneid]

fs throws a TypeError if path is not a string. However in basic functionality you can easly catch this error!

 fs = require('fs')
try{
fs.link(null, null, (err, data) => {
console.log("Errr", err)}) 
}catch(err){
console.log("Crash", err) // will emit Crash [TypeError: dest path must be a string]
}

code crash on fs.link in move/index.js. If you call wrap each try{} catch{} atomic async method (like fs.link) you will not given any of uncachted error., you can handle error it in async pattern (by callback)!. If you will not provide this protection, on low level, there is no chance to catch it on high level

Of course passing explicit null doesn't have sense, and i agree i can put any validation before run command. But this is not point. For me basic library which may crash whole application without chance to catch error, on such trivial level... It is something wrong

[RyanZim]

@jprichardson ?

I would tend to argue that in this case you should still type-check it yourself.

[enneid]

I think whole fs library throw only catchable errors.
Validation issue is not on this level. Library must not loose fully context and throw error outside app. In my opinion this is essencial issue in async programming

[jprichardson]

@enneid interested in submitting a PR?

[enneid]

I can't say when I may make pr , but ok