Using datejs causing 412 Precondition Failed errors

[ArcanoxDragon]

I recently re-installed my node_modules after a refactor, but without changing any versions in my package.json. I spent a few hours debugging an issue yesterday that I thought was part of my refactor, but it turned out to be a breaking change caused by serve-static's update to send@0.15.0. This version of send added support for the conditional request headers, which Chrome uses when requesting files that it has cached.

Currently, all of my static files that my browser has cached are being served as a 500 Internal Server Error because send sees that the precondition has failed (If-Modified-Since fails because the file has not been modified), causing my error handler to catch a PreconditionFailedError: Precondition Failed. send is supposed to return a proper response with a 412 status code in this case, but if there is a listener on the send stream's error event, it will instead throw the HTTP error as an error object via next(error). This bubbles down into the last-resort error handler for Express, which I have configured to return a 500 error code, and in development mode, a description of the error to the client.

I do not have fallthrough: false set on my call to express.static(...), and according to the documentation, this should cause the 412 error to fall through as a client error instead of letting it bubble through next(err) down into my last-resort error handler.

[dougwilson]

Weird, because If-Modified-Since is a 304 response (see our tests). Can you prove all the following:

1. Version of Node.js you are using
2. Complete code that we can run to reproduce the issue.
3. Complete instructions on how to reproduce.

Thanks!

[dougwilson]

Sorry, "provide all the following". Please forgive the phone typos.

[dougwilson]

And of course, PRs are always welcome and usually the easiest path to a solution :) so please feel free to make a PR you think is a good approach to solve the problem.

[ArcanoxDragon]

Let me see if I can get a minimal bit of code together to reproduce it and I'll see if I can find the root cause to PR it

[ArcanoxDragon]

Okay, so upon trying to get a minimal reproduction of the error, I found out that I couldn't. I then took out every piece of middleware in my main project except the static file middleware, and I still got the issue. After much debugging and commenting-out of code, I realized that it was my call to require("datejs") that is breaking send and thus serve-static. The error isn't caused by the precondition not being met, but rather by send not being compatible with datejs's Date.parse override. I suppose this is why everybody says not to use ambient modules.

I suppose I'll close the bug here, although I might open one with send or datejs, or just use a different date library...

[dougwilson]

Ah, I see. There is a tracking issue for this at https://github.com/expressjs/express/issues/3233 and the datejs directly to fix it is https://github.com/abritinthebay/datejs/issues/263

[ArcanoxDragon]

Ah, excellent! Thanks for searching for those ;)

I may still look for a different Date library that isn't an ambient module (tends to break the TypeScript paradigm, and it looks a bit odd to have a require() in the middle of all my TypeScript/ES6 import statements)

[ArcanoxDragon]

Just an afterthought...if this is due to DateJS returning null instead of NaN, does that mean Chrome is requesting an invalid date?

[dougwilson]

It could be. What is the header included in the requests that cause the failure?

[ArcanoxDragon]

The Chrome inspector tools show: If-Modified-Since: Sat, 04 Mar 2017 21:15:21 GMT

[dougwilson]

That date looks valid to me:

$ node -pe 'Date.parse("Sat, 04 Mar 2017 21:15:21 GMT")'
1488662121000

[dougwilson]

I am going to release a new patch version that will work-around the datejs issue.

[ChrisCinelli]

I think I have been impacted by this too:

PreconditionFailedError: Precondition Failed
    at SendStream.error (/var/app/current/node_modules/send/index.js:282:31)
    at SendStream.send (/var/app/current/node_modules/send/index.js:640:12)
    at onstat (/var/app/current/node_modules/send/index.js:737:10)
    at wrappedCallback 

[dougwilson]

Hi @ChrisCinelli sorry to hear you're having issues. The datejs was fixed in version 1.12.1. If you are using that version or higher, please file a new issue at https://github.com/expressjs/serve-static/issues with the version you're using and the exact code and steps to reproduce so we can dig in.