api: return 404 for alloc FS list/stat endpoints #11482
Conversation
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.
tgross
force-pushed
the
b-alloc-fs-404
branch
from
79fea58
to
7e8c154
Compare
tgross
requested review from
DerekStrickland and
lgfa29
and removed request for
lgfa29 and
DerekStrickland
|
This missed 1.2.0-rc1, so leaving this in draft state until after the 1.2.0 GA goes out. |
|
With the help of the UI team, I tracked down some UI related code and acceptance tests that I think will have to change or be deleted along with or shortly after this PR. cc @ChaiWithJai @lgfa29 |
|
I can follow-up with @ChaiWithJai and try my hand at a little bit of UI work 😀 |
I'm on it! |
Ember Asset Size actionAs of 5431400 Files that got Smaller 🎉:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
Ember Test Audit flaky testsEmber Test Audit detected these flaky tests on main:
Ember Test Audit detected these flaky tests on 70acdb6:
|
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.
ChaiWithJai
force-pushed
the
b-alloc-fs-404
branch
from
70acdb6
to
5431400
Compare
|
The UI changes LGTM but it's my PR so I can't be the one to approve it 😀 |
| @@ -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'); | |||
There was a problem hiding this comment.
These tests are a bit weird to me. Since the API now returns a proper 404, the endpoint stub should've been changed to return a 404 as well instead.
The two asserts are also kind of redundant. If one passes, the other will not fail since the error title can't be equal to two things at the same time.
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #11480
If the alloc filesystem doesn't have a file requested by the List Files (
/v1/client/fs/ls) or Stat File API (/v1/client/fs/stat), we currently return a HTTP 500 error with the expected "file not found" error message. Return a HTTP 404 error instead.This change doesn't help the streaming endpoints, but all our callers currently call the
/v1/client/fs/statendpoint first.