expressjs / expressjs/serve-index
How to handle fs.stat error on a file?
- Dominant language
- JavaScript
- Stars
- 447
- Forks
- 157
- PR merge metrics
- No merged PRs in 30d
Description
There are two potential error cases at present:
* ENOENT: `null` is returned rather than `stat`. This would cause an unhandled exception if it were ever encountered (i.e. the file is deleted between `fs.readdir` and `fs.stat`). However, it is unlikely that any one will ever encounter this error and much less likely that they will be able to repeat it to know what happened and submit a bug.
* Other errors (EPERM, etc). These are currently handled by passing the error to express and causing a 500 error.
ENOENT
----
In the first case I think we should just filter out the file.
If it's been deleted (or is otherwise a special type of file, such as a virtual file on fuse fs, which is returned from `fs.readdir()` but doesn't exist when you `fs.stat()`), why bother to show it? And why error out?
Other Erorrs
-----
I disagree with the current behavior. For one, it's inconsistent between text/plain and application/json responses (work) and text/html responses (fail).
I think we should instead provide a stat object that looks like this:
```
{ "name": "foo.txt"
, "size": 0
, "lastModified": "1970-01-01T00:00:00.000Z"
, "type": "error/EPERM"
}
```
(or maybe something more conventional like`type: application/vnd.eperm+x-error`)
Or perhaps omit the file.
In any case, I don't think that the current behavior of throwing a 500 on any single potential permission error is good behavior.
Contributor guide
Research direction
Start by tracing the fs.readdir and fs.stat handling for directory listings, then compare the text/plain, application/json, and text/html response paths. The issue presents several possible outcomes but no decision; done would require an agreed behavior for ENOENT and other stat errors, backed by tests for each response type.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- express, javascript, node.js
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100