webapi: Abort requests if web cache not ready.

[jholdstock]

A new middleware aborts any requests which require the data cache if the cache is not yet initialized. An explicit error is returned so the admin will be immediately aware what has gone wrong.

Previously, webpages were rendered and JSON responses were sent with zero values, and with no indication of anything being wrong. This was sometimes difficult to notice, and even when noticed the cause was not immediately apparent.

[jholdstock]

The cache failing to initialize shouldn't be a total showstopper - the vspd process should continue startup as normal so the rest of its functions can occur (tickets are adding to voting wallets etc). It's a recoverable error - chances are the cache will update sooner or later - e.g. once the RPC connection to dcrd is re-established.

As for why to use a middleware:

1. The .initialized() check and the associated error handling only have to exist in a single place rather than being duplicated N times in N different handlers (N is 7 currently).
2. Putting it first in the chain of middlewares means it will execute first - before hitting the database or any RPCs, before checking request authentication, etc. Failing faster in this fashion means clients will get their responses faster, and it will also lighten the load on the server.

[jholdstock]

Pushed one more commit which attempts to initialize the cache before returning an error.

[davecgh]

The cache failing to initialize shouldn't be a total showstopper - the vspd process should continue startup as normal so the rest of its functions can occur (tickets are adding to voting wallets etc). It's a recoverable error - chances are the cache will update sooner or later - e.g. once the RPC connection to dcrd is re-established.

Alright, got it. Thanks for the explanation. I understand and agree with the use of a middleware to require it with each request.
I'm just still not sure why it's being stored in the context though. Once requireWebCache has succeeded, either because the cache is already initialized or it wasn't already initialized but is able to successfully initialize it, all future calls to w.cache.getData() are guaranteed to succeed, correct?

Ergo, instead of c.Set(cacheKey, w.cache.getData()) followed up later by cacheData := c.MustGet(cacheKey).(cacheData), the handlers, which are all defined on the WebAPI type could safely do cachedData := w.cache.getData().

It's probably not a big deal, but I really don't like stuffing things into contexts in general when it can be avoided. It loses type safety and tends to hide dependencies.

EDIT: I'm fine with approving it as is if you disagree, btw. I don't see any issue that would prevent it from working properly. It's just more about trying to guard against potential future issues in refactors.

[jholdstock]

Your suggestion is correct, that would work and be safe. And while I do agree that the use of Context has significant drawbacks, in this particular case it actually does offer a small benefit in return. Retrieving from context implies that it must have been added to context - ie. it ensures that the middleware has been run and initialization has been checked. Accessing it directly offers no such guarantee, there would be no hard check that middleware has definitely been executed.

When context is used, middleware misconfiguration results in a development-time panic. When it isn't, the same misconfiguration results in a "silent failure" which can only be noticed by manual code review.

[davecgh]

The panic was actually precisely my concern (hence the comment about loss of type safety)! The cache isn't mandatory, but a slight configuration mistake will lead to the entire VSP going down at run time. However, I guess it wouldn't be too bad since the voting wallets are separate and thus it would only be the front end.