feat: add validation checks

[nfriedly]

This is a first stab at the validation checks I talked about in #356

There's more that I want to do, but wanted to get this bit up for now.

[gamemaker1]

Looks good so far :D

Linter doesn't seem very happy though, let me try fixing that.

[gamemaker1]

What if we made validation a boolean and allowed an onValidationError function to be passed with the options instead, so the user can decide what to do with the error?

[nfriedly]

What if we made validation a boolean and allowed an onValidationError function to be passed with the options instead, so the user can decide what to do with the error?

Yeah, related to that, it occurred to me that Throw will have different behavior for validations that run at startup (where throwing likely kills the server) vs in the context of a request (where throwing will be caught by express's error handler and the error message potentially passed to the end user in the HTTP response.) In light of that, I'm not sure I even want to support the Throw option.

What if validation had 3 potential values: false, a default function that calls console.warn, or a custom function. Although, then the question arises: should the custom function be expected to handle the request, or the regular path? And, should the regular rate-limiting still apply?

Honestly, that's starting to seem more confusing. Maybe we should just go with the validate setting being a boolean value for now: true = warn, false = skip all checks. We can always add a Throw option or an onValidationError setting later if there's demand for it.

---

On an unrelated note, I was thinking that each validation error should have a unique code, like ERR_ERL_INVALID_IP. We could set it in the error.code property, include it in the error message, and also include some information about it in our wiki. (Of course, error.code only matters if we throw or have a callback...)

[nfriedly]

Oh, and switching the config option to a boolean would also solve the "enum in the types file" confusion.

[gamemaker1]

That makes sense! Should I change it to that then?

[nfriedly]

That makes sense! Should I change it to that then?

Yeah, feel free. I'd like to do some more work on this, but I'm not sure when I'll get to it.

[gamemaker1]

Done.

[gamemaker1]

Although now we have an Error class in the types file :/

[nfriedly]

ok, decorators are out, runCheck is back in. (although I renamed it to wrap() since half it's job is to not run the checks. build looks to be passing now.

[nfriedly]

It just occurred to me that all of these checks are run per-request and can potentially log something on every single request. We might want to limit that. The ones here are probably fine to run on only the first request and none after.

Maybe we should include the stack trace to make it stand out more if we limit it to one log, though.

[nfriedly]

Ok, it now disables validations near the end of the first request, but logs a full stack trace if an issue is found.

I did a bit of testing and realized that the X-Forwarded-For check didn't work because the default trust proxy value is false. I updated things to account for this.

I also templatized the more info link part of the error message, since I initially forgot to update it when changing the error code.

I think it needs a little more testing (both manual and automated), and then I think it will be good to ship.

[gamemaker1]

This looks great!!

[nfriedly]

Ok, I think this is good to go, but lets wait a day or two to see if we want to do a point release with #361 first. I'm leaning towards that, but I'd really like to hear back that it resolves the issue first.

[nfriedly]

Alright, I think we've waited long enough. I'm going to ship this.

[SimonSchick]

Thank you for this, we did in-fact discover config issues due to this PR :)