mirror of
https://github.com/kemko/nomad.git
synced 2026-01-01 16:05:42 +03:00
api: return 404 for alloc FS list/stat endpoints (#11482)
* 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>
This commit is contained in:
3
.changelog/11482.txt
Normal file
3
.changelog/11482.txt
Normal file
@@ -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.
|
||||||
|
```
|
||||||
@@ -80,7 +80,7 @@ func (s *HTTPServer) DirectoryListRequest(resp http.ResponseWriter, req *http.Re
|
|||||||
}
|
}
|
||||||
|
|
||||||
if rpcErr != nil {
|
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())
|
rpcErr = CodedError(404, rpcErr.Error())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -120,7 +120,7 @@ func (s *HTTPServer) FileStatRequest(resp http.ResponseWriter, req *http.Request
|
|||||||
}
|
}
|
||||||
|
|
||||||
if rpcErr != nil {
|
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())
|
rpcErr = CodedError(404, rpcErr.Error())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -177,6 +177,10 @@ func IsErrNodeLacksRpc(err error) bool {
|
|||||||
return err != nil && strings.Contains(err.Error(), errNodeLacksRpc)
|
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
|
// NewErrRPCCoded wraps an RPC error with a code to be converted to HTTP status
|
||||||
// code
|
// code
|
||||||
func NewErrRPCCoded(code int, msg string) error {
|
func NewErrRPCCoded(code int, msg string) error {
|
||||||
|
|||||||
@@ -31,14 +31,8 @@ async function handleFSResponse(response) {
|
|||||||
} else {
|
} else {
|
||||||
const body = await response.text();
|
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 {
|
throw {
|
||||||
code: translatedCode,
|
code: response.status,
|
||||||
toString: () => body,
|
toString: () => body,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -361,7 +361,8 @@ export default function browseFilesystem({
|
|||||||
...visitSegments({ allocation: this.allocation, task: this.task }),
|
...visitSegments({ allocation: this.allocation, task: this.task }),
|
||||||
path: '/what-is-this',
|
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('/');
|
await visit('/');
|
||||||
|
|
||||||
@@ -385,7 +386,8 @@ export default function browseFilesystem({
|
|||||||
...visitSegments({ allocation: this.allocation, task: this.task }),
|
...visitSegments({ allocation: this.allocation, task: this.task }),
|
||||||
path: this.directory.name,
|
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('/');
|
await visit('/');
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user