[geometry] Meshcat's server returns 404 (not 200) on failures #20790
Conversation
jwnimmer-tri
added
priority: medium
release notes: fix
jwnimmer-tri
force-pushed
the
meshcat-url-handling
branch
from
4ea917c
to
6a0d3bf
Compare
|
+@SeanCurtis-TRI for feature review, please. |
jwnimmer-tri
force-pushed
the
meshcat-url-handling
branch
from
6a0d3bf
to
229caa1
Compare
There was a problem hiding this comment.
It seems that this PR is predominantly introducing and apply the ReadFile functions. Neither the PR title nor the commit even hints at it.
+@sherm1 for platform review.
Reviewed 21 of 21 files at r1, all commit messages.
Reviewable status: LGTM missing from assignee sherm1(platform)
|
To my eye, "start to carve helper functions" at least hints at it. I guess I could add the new function name to the commit body on the next rebase, if you think it helps? |
jwnimmer-tri
force-pushed
the
meshcat-url-handling
branch
from
229caa1
to
fbc2753
Compare
|
I found a lint error so I pushed a rebase anyway with a new note. |
There was a problem hiding this comment.
Nice cleanup! There are a few CI errors. Platform 
Reviewed 20 of 21 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion
geometry/meshcat_internal.h line 11 at r2 (raw file):
namespace internal { /* Returns the static content for the given URL, or an nullopt when the URL is
typo: "an nullopt" -> "a nullopt"
jwnimmer-tri
force-pushed
the
meshcat-url-handling
branch
from
fbc2753
to
3a5b93b
Compare
This is also an opportunity to start to carve helper functions and classes out into separate files for easier maintenance. Two of the new functions are public and widely relevant: ReadFile and ReadFileOrThrow.
jwnimmer-tri
force-pushed
the
meshcat-url-handling
branch
from
3a5b93b
to
fec399e
Compare
Towards #19598.
Returning cacheable 200 responses with empty bodies is seriously misleading to the browser-side code.
This change is