Possibly Trivial HTTPS Options Issue

[flcoder]

This is more of a suggestion than an issue. So, instead of having {cert: ***}, I had {crt: ***}, and there was no indication of anything amiss; the server started listening. Chrome was giving me ERR_SSL_VERSION_OR_CIPHER_MISMATCH. I googled my ass off and tried various things. There may be a reason to allow the options to have an unset cert property, but if there is none, perhaps we could have some kind of indication.

[silverwind]

I agree. We should throw if any of the required options to {tls,https}.createServer are missing.

[kulkarniankita]

I agree as well possibly a warning of the property not correctly spelled.

[ChALkeR]

@kulkarniankita That won't happen. Check if it is missing is fast and is a good thing to do (when the property is required), and trying to find a property with a similar name would require excess code and time, and is a no-go.

[kulkarniankita]

@ChALkeR Agreed good point. I can work on this if it gets decided to be implemented. (cc: @jasnell )

[jasnell]

@kulkarniankita ... +1 ... I agree with @silverwind, throwing if the required options are missing is a good idea. The only concern is that adding new throws is a semver-major but that's easily managed.

[kulkarniankita]

I have reproduced the issue, working on adding a fix for 'throwing if any of the required options to {tls,https}.createServer are missing'

[silverwind]

👍

[Trott]

I'm going to remove the good first contribution label as this is a fair bit more complicated than it appears at first blush. For starters, there are ciphers that do not require key/cert, so merely throwing an error if key or cert is missing is not correct.

It appears that there is a good start on this at #3064 but that PR got stalled. Pinging @kulkarniankita to see if maybe she's ready to pick it back up and finish it off, because that would be awesome.

[flcoder]

@Trott In the case of possible ciphers not using key or cert I might suggest then to delegate the throw logic to whatever consumers(ciphers in this case) are dependent upon the option. Ciphers can designate what options they require and the initial options handler can then validate the options. In any case, I feel the rule of thumb is this: if a consumer of options requires a number of those options to function properly, those required options are no longer optional. They may start out as options when provided to the first handler, but once the handler delivers them to the end-consumer (cipher), they could turn into required arguments. My final input is this... if it is a case where performance were an issue, like inside a loop, such as a rendering loop, then of course you're not going to be checking properties and doing validations just to help out the coder, but within a process that is generally within initialization steps and only being done once or a few times, I think, even if a bit of refactoring is required, it's definitely worth X minutes of lib dev's time and Y nanoseconds of options validation time in order to save Z (minutes/hours/days x number of lib consuming devs) time. In the end, no one will thank you... because when the exception is thrown, they'll fix their code and move on without ever realizing that some lib dev saved them a lot of time, but I guess the upside is that no one will growl at you or worse, lose faith in your lib. I should probably go take an anti OCD pill now, but I'll end with saying, good work overall, node is awesome and has changed my life.

[kulkarniankita]

@Trott I can pick it up as I would like to finish what I started!

[Trott]

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.