Skip to content

Commit

Permalink
api: return 404 for alloc FS list/stat endpoints (#11482)
Browse files Browse the repository at this point in the history
* api: return 404 for alloc FS list/stat endpoints

If the alloc filesystem doesn't have a file requested by the List
Files or Stat File API, we currently return a HTTP 500 error with the
expected "file not found" error message. Return a HTTP 404 error
instead.

* update FS Handler

Previously the FS handler would interpret a 500 status as a 404
in the adapter layer by checking if the response body contained
the text  or is the response status
was 500 and then throw an error code for 404.

Co-authored-by: Jai Bhagat <jaybhagat841@gmail.com>
  • Loading branch information
tgross and ChaiWithJai committed Nov 17, 2021
1 parent df41bde commit 55c29fb
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 11 deletions.
3 changes: 3 additions & 0 deletions .changelog/11482.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
api: Return a HTTP 404 instead of a HTTP 500 from the Stat File and List Files API endpoints when a file or directory is not found.
```
4 changes: 2 additions & 2 deletions command/agent/fs_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (s *HTTPServer) DirectoryListRequest(resp http.ResponseWriter, req *http.Re
}

if rpcErr != nil {
if structs.IsErrNoNodeConn(rpcErr) || structs.IsErrUnknownAllocation(rpcErr) {
if structs.IsErrNoNodeConn(rpcErr) || structs.IsErrUnknownAllocation(rpcErr) || structs.IsErrNoSuchFileOrDirectory(rpcErr) {
rpcErr = CodedError(404, rpcErr.Error())
}

Expand Down Expand Up @@ -120,7 +120,7 @@ func (s *HTTPServer) FileStatRequest(resp http.ResponseWriter, req *http.Request
}

if rpcErr != nil {
if structs.IsErrNoNodeConn(rpcErr) || structs.IsErrUnknownAllocation(rpcErr) {
if structs.IsErrNoNodeConn(rpcErr) || structs.IsErrUnknownAllocation(rpcErr) || structs.IsErrNoSuchFileOrDirectory(rpcErr) {
rpcErr = CodedError(404, rpcErr.Error())
}

Expand Down
4 changes: 4 additions & 0 deletions nomad/structs/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ func IsErrNodeLacksRpc(err error) bool {
return err != nil && strings.Contains(err.Error(), errNodeLacksRpc)
}

func IsErrNoSuchFileOrDirectory(err error) bool {
return err != nil && strings.Contains(err.Error(), "no such file or directory")
}

// NewErrRPCCoded wraps an RPC error with a code to be converted to HTTP status
// code
func NewErrRPCCoded(code int, msg string) error {
Expand Down
8 changes: 1 addition & 7 deletions ui/app/adapters/allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,8 @@ async function handleFSResponse(response) {
} else {
const body = await response.text();

// TODO update this if/when endpoint returns 404 as expected
const statusIs500 = response.status === 500;
const bodyIncludes404Text = body.includes('no such file or directory');

const translatedCode = statusIs500 && bodyIncludes404Text ? 404 : response.status;

throw {
code: translatedCode,
code: response.status,
toString: () => body,
};
}
Expand Down
6 changes: 4 additions & 2 deletions ui/tests/acceptance/behaviors/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,8 @@ export default function browseFilesystem({
...visitSegments({ allocation: this.allocation, task: this.task }),
path: '/what-is-this',
});
assert.equal(FS.error.title, 'Not Found', '500 is interpreted as 404');
assert.notEqual(FS.error.title, 'Not Found', '500 is not interpreted as 404');
assert.equal(FS.error.title, 'Server Error', '500 is not interpreted as 500');

await visit('/');

Expand All @@ -385,7 +386,8 @@ export default function browseFilesystem({
...visitSegments({ allocation: this.allocation, task: this.task }),
path: this.directory.name,
});
assert.equal(FS.error.title, 'Not Found', '500 is interpreted as 404');
assert.notEqual(FS.error.title, 'Not Found', '500 is not interpreted as 404');
assert.equal(FS.error.title, 'Server Error', '500 is not interpreted as 404');

await visit('/');

Expand Down

0 comments on commit 55c29fb

Please sign in to comment.