Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling Stat on a non existent file returns 500 rather than 404 #11480

Closed
tomqwpl opened this issue Nov 9, 2021 · 4 comments · Fixed by #11482
Closed

Calling Stat on a non existent file returns 500 rather than 404 #11480

tomqwpl opened this issue Nov 9, 2021 · 4 comments · Fixed by #11482

Comments

@tomqwpl
Copy link

tomqwpl commented Nov 9, 2021

Nomad version

v1.1.4

Operating system and Environment details

Windows 10

Issue

I'm trying to test for the existence of a file in an allocation directory.
I call:

nomadClient.AllocFS().Stat(alloc, filename)

If the file doesn't exist, this returns an error "Unexpected response code: 500".
Internal server error 500 isn't really an expected response to this, a 404 would be more expected.

Note that in general I seem to have to match the text of the error to detect the 404 case elsewhere, rather than there being a specific error type I can test for for example. Matching on the string of the error text feels fragile, but at least matching for a specific string containing 404 would be preferable to a more generic 500.

Reproduction steps

Expected Result

An error I can use to easily detect "not found"

Actual Result

An ambiguous error that could mean many things

@tgross tgross added this to Needs Triage in Nomad - Community Issues Triage via automation Nov 9, 2021
@tgross
Copy link
Member

tgross commented Nov 9, 2021

Hi @tomqwpl! Ok, so I followed that code path and it hits the HTTP API, where it calls the server RPC, which calls the client RPC, which calls into the alloc dir functions that calls os.Stat (which is the operating system interface).

What we should be doing is capturing the error from the allocdir package at client/fs_endpoint.go#L148-L151 and converting that to a 404 if it's a "path not found" error. There'll also be an interaction with the sandbox check we need to think about.

@tgross tgross self-assigned this Nov 9, 2021
@tgross tgross moved this from Needs Triage to In Progress in Nomad - Community Issues Triage Nov 9, 2021
@tgross tgross added this to the 1.2.1 milestone Nov 9, 2021
Nomad - Community Issues Triage automation moved this from In Progress to Done Nov 17, 2021
@tgross
Copy link
Member

tgross commented Nov 17, 2021

I've shipped a fix as #11482 and that'll ship in the next regular patch version of Nomad.

@tomqwpl
Copy link
Author

tomqwpl commented Nov 17, 2021

Cool. Thx.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging a pull request may close this issue.

2 participants