Deno.listen(":8000") gives an indecipherable error message

[ry]

Deno.listen(":8000") is missing the first argument - it should be Deno.listen("tcp", ":8000"). The error message for this common mistake looks scary and doesn't give a good idea of what's happening:

Uncaught TypeError: Cannot read property 'length' of undefined
    at flatbuffers.Builder.createString (third_party/node_modules/flatbuffers/js/flatbuffers.mjs:747:18)
    at listen (js/net.ts:142:28)
    at main (file:///Users/rld/deno_examples/echo_server.js:4:16)
    at file:///Users/rld/deno_examples/echo_server.js:9:1

[kevinkassimo]

This only happens in JS files and it is due to no runtime validation of arguments (types & argument list length). I brought up this once before but Kitson was strongly against adding these checks due to performance concerns.

[zekth]

But it's only on the initialization of the ListenerImpl so doing null check is not affecting performances of the http server or am i wrong?

[incadenza]

Yeah, seems like the checking could be done prior to initializing the ListenerImpl class. I'm interested in the project and happy to take this on if there are no additional concerns.

After just looking for a moment, worth noting that the same issue likely applies to to dial as well.

[hayd]

The issue is that it applies to every op. I think that was @kitsonk 's perf concern, also this is going to potentially be a lot of boilerplate code wrapping every op.

[incadenza]

It certainly would be. Correct me if I'm wrong, but it's not clear what choice there is. Either allow .js files and throw un-friendly errors, or provide some friendlier run time validation errors, no?

[kitsonk]

My concern has always been consistency in guarding against parameters at runtime and maintaining it considering the fact that we have TypeScript which does all of that essentially at runtime in Deno. My opinion is people choosing to abuse APIs in JavaScript who can get meaningful, self maintaining type errors get what they deserve. Restoring CheckJs would also allow this without the use of TS to get meaningful errors. That should be possible after we land the tsconfig. So that is my opinion, we should leverage that platform we have.

[Swivelgames]

@kitsonk So, in other words, if you write a piece of code in JavaScript and get a runtime error, you deserve it because you weren't using TypeScript? Sounds more like Deno supports it because it has to. Why support it? Just amputate the infected limb and make it TypeScript-only. Doesn't sound like its worth the extra effort and degradation in performance.

For what it's worth, if someone provides a public API, it should probably be optimized for use in the avenues its available in. If it's not worth the effort or performance trade-offs, then supporting it in a degraded state is bad for both parties. That includes giving meaningful errors. The error in question above is sending people on wild goose chases simply because it's not worth degrading performance. Seems a bit silly.

[kitsonk]

@swivelgames I think you are misunderstanding my opinion. Runtime argument checking is always chasing your tail in JavaScript. There are infinite numbers of ways of abusing APIs in JavaScript. Check JS feature would immediately give a meaningful error in this case, but we don't enable it at the moment.

Runtime argument type checking in JavaScript is a rabbit hole. I am just challenging the received wisdom of throwing a bunch of runtime type checking into a platform that has a much better and more accurate way of checking arguments that will always give more meaningful errors.

It us just an opinion to make people think, versus an immutable opinion.

[diasbruno]

Here is one idea to solve this. If we can flip the arguments of listen() and dial(), and assuming Network can be "tcp" by default, perhaps this interface can help avoid misusage and avoid introducing runtime verification.

function listen(address: string, network: Network = "tcp"): Listener;
function dial(address: string, network: Network = "tcp"): Promise<Conn>

[bartlomieju]

This issue is not relevant anymore with new Deno.listen({ transport, hostname, port }) API

CC @ry