Is there consensus on the value to pass to a callback if there isn't
an error and there may or may not be a resulting value. A quick survey
of node.js code along with popular modules seems to indicate that null
is most common but undefined and false also pop up frequently. While
any falsey value works I was curious if one was preferred.
It should be null or undefined only, everything else is a value, and can mistakenly accepted as an error.
There are however asynchronous functions that never resolve with an error (e.g. path.exists) and as there's no point in always sending null to their callbacks, they're designed to pass success value as first argument (in case of path.exists true or false).
On Wednesday, May 30, 2012 3:24:06 AM UTC+2, Daniel R. wrote:
> Is there consensus on the value to pass to a callback if there isn't > an error and there may or may not be a resulting value. A quick survey > of node.js code along with popular modules seems to indicate that null > is most common but undefined and false also pop up frequently. While > any falsey value works I was curious if one was preferred.
The most correct value for the error parameter is "undefined", but most of
us are lazy and type "null" instead. In most code it expects the err
parameter to be either falsy or an Error instance, so it doesn't matter if
you use false, undefined, null, 0, or even "".
While there are some functions that never emit an error, if there is any
chance they would emit one in the future, save yourself the trouble and
reserve the err parameter. Also it makes the function work with
flow-control tools and wrappers that assume the pattern.
On Wed, May 30, 2012 at 4:05 AM, Mariusz Nowak <mari...@medikoo.com> wrote:
> It should be null or undefined only, everything else is a value, and can
> mistakenly accepted as an error.
> There are however asynchronous functions that never resolve with an error
> (e.g. path.exists) and as there's no point in always sending null to their
> callbacks, they're designed to pass success value as first argument (in
> case of path.exists true or false).
> --
> Mariusz Nowak - @medikoo
> On Wednesday, May 30, 2012 3:24:06 AM UTC+2, Daniel R. wrote:
>> Is there consensus on the value to pass to a callback if there isn't
>> an error and there may or may not be a resulting value. A quick survey
>> of node.js code along with popular modules seems to indicate that null
>> is most common but undefined and false also pop up frequently. While
>> any falsey value works I was curious if one was preferred.
path.exists (moved to fs.exists) is an abomination for so many
reasons. We can't get rid of it, because it's too widely used, but
it's really pretty awful. It predates the cb(er, data) pattern in
node, and is one of the very few exceptions to it.
Any time you find yourself using it, you should try to figure out what
you're really trying to find out, and use some other fs call instead.
Usually the best approach is to just try to do what you're going to
do, and handle the error. If you're using a file's existence as a
lock of some sort, it's better to open the file in O_EXCL mode
instead.
On Wed, May 30, 2012 at 7:37 AM, Tim Caswell <t...@creationix.com> wrote:
> The most correct value for the error parameter is "undefined", but most of
> us are lazy and type "null" instead. In most code it expects the err
> parameter to be either falsy or an Error instance, so it doesn't matter if
> you use false, undefined, null, 0, or even "".
> While there are some functions that never emit an error, if there is any
> chance they would emit one in the future, save yourself the trouble and
> reserve the err parameter. Also it makes the function work with
> flow-control tools and wrappers that assume the pattern.
> On Wed, May 30, 2012 at 4:05 AM, Mariusz Nowak <mari...@medikoo.com> wrote:
>> It should be null or undefined only, everything else is a value, and can
>> mistakenly accepted as an error.
>> There are however asynchronous functions that never resolve with an error
>> (e.g. path.exists) and as there's no point in always sending null to their
>> callbacks, they're designed to pass success value as first argument (in case
>> of path.exists true or false).
>> --
>> Mariusz Nowak - @medikoo
>> On Wednesday, May 30, 2012 3:24:06 AM UTC+2, Daniel R. wrote:
>>> Is there consensus on the value to pass to a callback if there isn't
>>> an error and there may or may not be a resulting value. A quick survey
>>> of node.js code along with popular modules seems to indicate that null
>>> is most common but undefined and false also pop up frequently. While
>>> any falsey value works I was curious if one was preferred.
I don't know why we can't change it. Sure it may break a lot of working code. But as you've outlined, replacing it with more preferable methods is fairly straight forward. It's even easily shimmable for folks who don't want to upgrade. We're talking about breaking nextTick but "can't" get rid of this? Doesn't seem logical.
On Wednesday, May 30, 2012 9:22:42 AM UTC-7, Isaac Schlueter wrote:
> path.exists (moved to fs.exists) is an abomination for so many > reasons. We can't get rid of it, because it's too widely used, but > it's really pretty awful. It predates the cb(er, data) pattern in > node, and is one of the very few exceptions to it.
> Any time you find yourself using it, you should try to figure out what > you're really trying to find out, and use some other fs call instead. > Usually the best approach is to just try to do what you're going to > do, and handle the error. If you're using a file's existence as a > lock of some sort, it's better to open the file in O_EXCL mode > instead.
> On Wed, May 30, 2012 at 7:37 AM, Tim Caswell <t...@creationix.com> wrote: > > The most correct value for the error parameter is "undefined", but most > of > > us are lazy and type "null" instead. In most code it expects the err > > parameter to be either falsy or an Error instance, so it doesn't matter > if > > you use false, undefined, null, 0, or even "".
> > While there are some functions that never emit an error, if there is any > > chance they would emit one in the future, save yourself the trouble and > > reserve the err parameter. Also it makes the function work with > > flow-control tools and wrappers that assume the pattern.
> > On Wed, May 30, 2012 at 4:05 AM, Mariusz Nowak <mari...@medikoo.com> > wrote:
> >> It should be null or undefined only, everything else is a value, and > can > >> mistakenly accepted as an error.
> >> There are however asynchronous functions that never resolve with an > error > >> (e.g. path.exists) and as there's no point in always sending null to > their > >> callbacks, they're designed to pass success value as first argument (in > case > >> of path.exists true or false).
> >> -- > >> Mariusz Nowak - @medikoo
> >> On Wednesday, May 30, 2012 3:24:06 AM UTC+2, Daniel R. wrote:
> >>> Is there consensus on the value to pass to a callback if there isn't > >>> an error and there may or may not be a resulting value. A quick survey > >>> of node.js code along with popular modules seems to indicate that null > >>> is most common but undefined and false also pop up frequently. While > >>> any falsey value works I was curious if one was preferred.
If you're using a pre-1.0 platform, your code is subject to breakage. Also you have to opt into upgrading and every major node update has required minor changes.
At some point we went from changing the node api a lot to make it better to not changing it at all for fear of crying developers. I'm not suggesting we open flood gates. I'm suggesting we continue to refine the api and get as close to one with no unnecessary warts as we can before going 1.0. Because then we really are screwed.
> If you're using a pre-1.0 platform, your code is subject to breakage. Also
> you have to opt into upgrading and every major node update has required
> minor changes.
> At some point we went from changing the node api a lot to make it better
> to not changing it at all for fear of crying developers. I'm not suggesting
> we open flood gates. I'm suggesting we continue to refine the api and get
> as close to one with no unnecessary warts as we can before going 1.0.
> Because then we really are screwed.
> :Marco
> On Wednesday, May 30, 2012 3:45:53 PM UTC-7, Mark Hahn wrote:
>> I would be very unhappy if someone broke my code. Ask the jquery bdfl
>> what happened when he tried to make a non-backwards-compatible change.
On Wed, May 30, 2012 at 5:11 PM, Mark Hahn <m...@hahnca.com> wrote:
> The ver 1.0 moniker is pretty much meaningless. Many many people have it
> in production.
> On Wed, May 30, 2012 at 4:49 PM, Marco Rogers <marco.rog...@gmail.com>wrote:
>> If you're using a pre-1.0 platform, your code is subject to breakage.
>> Also you have to opt into upgrading and every major node update has
>> required minor changes.
>> At some point we went from changing the node api a lot to make it better
>> to not changing it at all for fear of crying developers. I'm not suggesting
>> we open flood gates. I'm suggesting we continue to refine the api and get
>> as close to one with no unnecessary warts as we can before going 1.0.
>> Because then we really are screwed.
>> :Marco
>> On Wednesday, May 30, 2012 3:45:53 PM UTC-7, Mark Hahn wrote:
>>> I would be very unhappy if someone broke my code. Ask the jquery bdfl
>>> what happened when he tried to make a non-backwards-compatible change.
Yes I know that. But that is despite the pre-1.0 version. Node has tried to be very strict about what he version means from the beginning. pre-1.0 means "stable, but not done". Some people think "stable" means "we won't break backwards compat". But I'm suggesting that it actually mean "we'll try really hard for backwards compat, but we might break things for the good of the api". I think in the case of path.exists, this is very doable. If we can't do stuff like this now, then we are resigning ourselves to being stuck with these warts forever. And path.exists/fs.exists is a particular bad wart because it's the exact place people will look for this functionality. So the propagation of this will continue to go up.
On Wednesday, May 30, 2012 5:11:37 PM UTC-7, Mark Hahn wrote:
> It's like google's "beta" gmail was.
> On Wed, May 30, 2012 at 5:11 PM, Mark Hahn <m...@hahnca.com> wrote:
>> The ver 1.0 moniker is pretty much meaningless. Many many people have it >> in production.
>> On Wed, May 30, 2012 at 4:49 PM, Marco Rogers <marco.rog...@gmail.com>wrote:
>>> If you're using a pre-1.0 platform, your code is subject to breakage. >>> Also you have to opt into upgrading and every major node update has >>> required minor changes.
>>> At some point we went from changing the node api a lot to make it better >>> to not changing it at all for fear of crying developers. I'm not suggesting >>> we open flood gates. I'm suggesting we continue to refine the api and get >>> as close to one with no unnecessary warts as we can before going 1.0. >>> Because then we really are screwed.
>>> :Marco
>>> On Wednesday, May 30, 2012 3:45:53 PM UTC-7, Mark Hahn wrote:
>>>> I would be very unhappy if someone broke my code. Ask the jquery bdfl >>>> what happened when he tried to make a non-backwards-compatible change.
> Yes I know that. But that is despite the pre-1.0 version. Node has tried
> to be very strict about what he version means from the beginning. pre-1.0
> means "stable, but not done". Some people think "stable" means "we won't
> break backwards compat". But I'm suggesting that it actually mean "we'll
> try really hard for backwards compat, but we might break things for the
> good of the api". I think in the case of path.exists, this is very doable.
> If we can't do stuff like this now, then we are resigning ourselves to
> being stuck with these warts forever. And path.exists/fs.exists is a
> particular bad wart because it's the exact place people will look for this
> functionality. So the propagation of this will continue to go up.
> :Marco
> On Wednesday, May 30, 2012 5:11:37 PM UTC-7, Mark Hahn wrote:
>> It's like google's "beta" gmail was.
>> On Wed, May 30, 2012 at 5:11 PM, Mark Hahn <m...@hahnca.com> wrote:
>>> The ver 1.0 moniker is pretty much meaningless. Many many people have
>>> it in production.
>>> On Wed, May 30, 2012 at 4:49 PM, Marco Rogers <marco.rog...@gmail.com>wrote:
>>>> If you're using a pre-1.0 platform, your code is subject to breakage.
>>>> Also you have to opt into upgrading and every major node update has
>>>> required minor changes.
>>>> At some point we went from changing the node api a lot to make it
>>>> better to not changing it at all for fear of crying developers. I'm not
>>>> suggesting we open flood gates. I'm suggesting we continue to refine the
>>>> api and get as close to one with no unnecessary warts as we can before
>>>> going 1.0. Because then we really are screwed.
>>>> :Marco
>>>> On Wednesday, May 30, 2012 3:45:53 PM UTC-7, Mark Hahn wrote:
>>>>> I would be very unhappy if someone broke my code. Ask the jquery bdfl
>>>>> what happened when he tried to make a non-backwards-compatible change.
I looked at the Node's source, and indeed path.exists (to be fs.exists) is just wrapper over fs.stat.
If there's no "lighter" way for checking whether path exists, indeed it's best it it's not in the API at all. There are already two good reasons for that: it encourages bad practices and it's asynchronous function that doesn't follow convention.
On Thu, May 31, 2012 at 8:08 PM, Marco Rogers <marco.rog...@gmail.com> wrote:
>> On a more serious note, what harm does .exists do? http.createServer
>> violates all the rules and it is the first thing a node newbie sees.
> Curious. What rules you think createServer violates?
The callback gets no error argument as the first argument.
I would rigorously throw away _all_ of those inconsistencies right
now, but at least with 0.8. For me, the leading zero in a
version-number means: Expect changes, anytime, everywhere.
Nobody could foresee that it is generally best practice to always
reserve the first argument of an callback for error-objects, no matter
if the function checks for file-existence or if it creates a
http-server. But everyone decided to run node.js in production before
the first major release, version 1.0.0. Everyone should know about
that and don't wine if anything breaks.
Change it before 1. But I doubt that this will happen.
BTW: That google-mail 'beta' label was pure marketing.
@Oliver createServer is not asynchronous function, it is synchronous function that returns event emitter and optionally accepts listener. It's different concept/pattern and all rules are followed
On Thursday, May 31, 2012 9:08:03 PM UTC+2, Oliver Leics wrote:
> On Thu, May 31, 2012 at 8:08 PM, Marco Rogers <marco.rog...@gmail.com> > wrote:
> >> On a more serious note, what harm does .exists do? http.createServer > >> violates all the rules and it is the first thing a node newbie sees.
> > Curious. What rules you think createServer violates?
> The callback gets no error argument as the first argument.
> I would rigorously throw away _all_ of those inconsistencies right > now, but at least with 0.8. For me, the leading zero in a > version-number means: Expect changes, anytime, everywhere.
> Nobody could foresee that it is generally best practice to always > reserve the first argument of an callback for error-objects, no matter > if the function checks for file-existence or if it creates a > http-server. But everyone decided to run node.js in production before > the first major release, version 1.0.0. Everyone should know about > that and don't wine if anything breaks.
> Change it before 1. But I doubt that this will happen.
> BTW: That google-mail 'beta' label was pure marketing.
On Thu, May 31, 2012 at 9:15 PM, Mariusz Nowak <mari...@medikoo.com> wrote:
> @Oliver createServer is not asynchronous function, it is synchronous
> function that returns event emitter and optionally accepts listener. It's
> different concept/pattern and all rules are followed
I know that, but the missing error parameter for the callback is still
an inconsistency (even if there are very very rare till zero cases
that actually give an error-object on req-res connection level)
On Thursday, May 31, 2012 9:22:05 PM UTC+2, Oliver Leics wrote:
> On Thu, May 31, 2012 at 9:15 PM, Mariusz Nowak <mari...@medikoo.com> > wrote: > > @Oliver createServer is not asynchronous function, it is synchronous > > function that returns event emitter and optionally accepts listener. > It's > > different concept/pattern and all rules are followed
> I know that, but the missing error parameter for the callback is still > an inconsistency (even if there are very very rare till zero cases > that actually give an error-object on req-res connection level)
On Thu, May 31, 2012 at 9:26 PM, Mariusz Nowak <mari...@medikoo.com> wrote:
> @Oliver Synchronous functions throw errors if there's any, they do not take
> any callback for that.
You can repeat that "it's a synchronous function" argument as long as
you want, for me it is an inconsistency that the callback of
http.createServer gets no error argument (even if rarely or never
used)
BTW: http.createServer should/must continue to emit the error event.
That's very consistent.
On Thu, May 31, 2012 at 9:39 PM, Oliver Leics <oliver.le...@gmail.com> wrote:
> On Thu, May 31, 2012 at 9:26 PM, Mariusz Nowak <mari...@medikoo.com> wrote:
>> @Oliver Synchronous functions throw errors if there's any, they do not take
>> any callback for that.
> You can repeat that "it's a synchronous function" argument as long as
> you want, for me it is an inconsistency that the callback of
> http.createServer gets no error argument (even if rarely or never
> used)
On Thu, May 31, 2012 at 3:39 PM, Oliver Leics <oliver.le...@gmail.com>wrote:
> You can repeat that "it's a synchronous function" argument as long as
> you want, for me it is an inconsistency that the callback of
> http.createServer gets no error argument (even if rarely or never
> used)
The point is that it's not a callback. It's a shortcut for adding a request
event listener.
<scott.gonza...@gmail.com> wrote:
> On Thu, May 31, 2012 at 3:39 PM, Oliver Leics <oliver.le...@gmail.com>
> wrote:
>> You can repeat that "it's a synchronous function" argument as long as
>> you want, for me it is an inconsistency that the callback of
>> http.createServer gets no error argument (even if rarely or never
>> used)
> The point is that it's not a callback. It's a shortcut for adding a request
> event listener.
Yes, I know that too. And you might have already guessed it: It's just
another inconsistency that confuses ppl ;-)
> Yes, I know that too. And you might have already guessed it: It's just
> another inconsistency that confuses ppl ;-)
It's not inconsistent. The server is an event emitter the error handling
pattern is server.on("error", errorHandler). There are two error handling
patterns.
The other error handling pattern of (err, data) is for asynchronous
operations that are not event emitters.
One can argue that the macro of having http.createServer(f) be
http.createServer().on("request", f) is confusing
