UnhandledPromiseRejectionWarning thrown if config.staticFiles.path contains a directory with the same name as a routable state's uri

[data-doge]

interesting bug that appeared while refactoring GC's hotline code:

• i set twilio-ivr's config.staticFiles.path to public
• inside of public, there's a directory named operator-unavailable.
• i created a RoutableState with uri /operator-unavailable
• in a different state, i try to construct a url urlFor('/operator-unavailable', { absolute: true })
• this throws an error:

Error: EISDIR: illegal operation on a directory, read
    at Error (native)
    at Object.fs.readSync (fs.js:731:19)
    at tryReadSync (fs.js:486:20)
    at Object.fs.readFileSync (fs.js:534:19)
    at Object.md5 [as fingerprint] (/Users/eugenelynch/code/good-call/hotline/node_modules/twilio-interactive-flow/node_modules/static-expiry/index.js:84:15)
    at fingerprintAssetUrl (/Users/eugenelynch/code/good-call/hotline/node_modules/twilio-interactive-flow/node_modules/static-expiry/index.js:153:29)
    at furl (/Users/eugenelynch/code/good-call/hotline/node_modules/twilio-interactive-flow/node_modules/static-expiry/index.js:225:55)
    at /Users/eugenelynch/code/good-call/hotline/node_modules/twilio-interactive-flow/build/lib/util/staticExpiryHelpers.js:44:28
    at urlFingerprinter (/Users/eugenelynch/code/good-call/hotline/node_modules/twilio-interactive-flow/build/lib/index.js:60:25)
    at e (/Users/eugenelynch/code/good-call/hotline/node_modules/twilio-interactive-flow/build/lib/util/routeCreationHelpers.js:60:44)
    at Object.backgroundTrigger (/Users/eugenelynch/code/good-call/hotline/build/src/states/userCall/CONNECT_TO_OPERATOR.js:22:32)
    at renderableStatePromise.then.stateToRender (/Users/eugenelynch/code/good-call/hotline/node_modules/twilio-interactive-flow/build/lib/util/routeCreationHelpers.js:27:27)
    at process._tickCallback (internal/process/next_tick.js:103:7)

• though, changing said RoutableState's uri to /operator-unavailable-1, and using urlFor('/operator-unavailable-1', { absolute: true }) to construct the url works just fine.

[ethanresnick]

interesting indeed.

a quick fix is to call urlFor with the fingerprint option set to false, i.e.:

urlFor('/operator-unavailable', { absolute: true, fingerprint: false })

By default the fingerprint option of the urlFor function is set to true (unless the query option is used), because most of the time urlFor is used to produce fingerprinted static file urls. Whenever you're not intending to generate such a url, setting fingerprint to false is technically the correct thing to do. That said, this shouldn't blow up (at least not exactly this way) even if you forget to do that. I'll write up a more in-depth comment later about the underlying bug and possible fixes.

[ethanresnick]

ok, some more details on some of the inter-related problems here. I'm spelling this out mostly for myself.

1. config.staticFiles.mountPath determines what path in the url the static files will live "under". If it's empty, as I think it is in your example, then that's saying that the static files should be served right from the root url. (I.e.,GET /${staticFileName} should respond with the file at public/${staticFileName}). So, with an empty mountPath, /operator-unavailable is actually ambiguous: it could refer to the routable state, or to a directory or (extensionless) file called operator-unavailable that's in the public directory. I think, if you request /operator-unavailable, you'll actually get back the routable state, but that's only because directories aren't serveable directly, so they're skipped. (Likewise, missing files are skipped, which is how the other routable states get served.)

2. However, if operator-unavailable were the name of an file under the static files path, rather than a directory, then what should be served (that file or the routable state) would be undefined. All of which is to say: one is always gonna have to either watch for name collisions to make sure the right thing is served, or, more safely, define a static files mount path, like /static, so that there's no risk of collision (as long as /static isn't a routable state's url). I think the hotline does the former atm, but should probably switch to the latter.

3. urlFor is blowing up because it's trying to fingerprint the provided url, as it does by default. To do that, though, it looks for the file that the url could correspond to (based on the config) and tries to hash that file's contents. Because in this case the url (when it's being interpreted as related to static files for fingerprinting) corresponds to a directory, shit blows up, because a directory doesn't have contents to hash like a file does to create the fingerprint.

4. So, one question is what to do when the url being fingerprinted points to a directory (which could happen even with a mountPath). Since a directory itself can't be served (unless some index file at that directory is served in its place, which we're not doing atm), blowing up in some form is probably correct, but we could have a better error message.

5. Finally, there's the question of whether we could make urlFor a little smarter, so that the fingerprint option has a better default. The one thing I'm thinking here is that we could create the urlFor function with knowledge of the static files mount path, so that it only tries to fingerprint urls by default that are underneath that path. So, the default for fingerprint would change from "fingerprint iff [a fingerprint function is provided && the query option is not used]" to "fingerprint iff a [fingerprint function is provided && ((there's a static files mount path && this url is under that path) || the query option is not used)]". That's probably a little more user friendly — although it doesn't help in your example, because there's no mount path. However, I wonder whether a default like that starts to make the fingerprint default seem a little too "magical". Thoughts?