Skip to content

Commit

Permalink
[geometry] Meshcat's server returns 404 (not 200) on failures
Browse files Browse the repository at this point in the history
This is also an opportunity to start to carve helper functions and
classes out into separate files for easier maintenance.
  • Loading branch information
jwnimmer-tri committed Jan 17, 2024
1 parent 0ba6402 commit 6a0d3bf
Show file tree
Hide file tree
Showing 21 changed files with 244 additions and 170 deletions.
12 changes: 12 additions & 0 deletions bindings/pydrake/geometry/test/visualizers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,18 @@ def test_meshcat(self):
# The pose is None because no meshcat session has broadcast its pose.
self.assertIsNone(meshcat.GetTrackedCameraPose())

def test_meshcat_404(self):
meshcat = mut.Meshcat()

good_url = meshcat.web_url()
with urllib.request.urlopen(good_url) as response:
self.assertTrue(response.read(1))

bad_url = f"{good_url}/no_such_file"
with self.assertRaisesRegex(Exception, "HTTP.*404"):
with urllib.request.urlopen(bad_url) as response:
response.read(1)

def test_meshcat_animation(self):
animation = mut.MeshcatAnimation(frames_per_second=64)
self.assertEqual(animation.frames_per_second(), 64)
Expand Down
1 change: 1 addition & 0 deletions common/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,7 @@ drake_cc_googletest(
":drake_path",
":find_resource",
"//common/test_utilities:expect_no_throw",
"//common/test_utilities:expect_throws_message",
],
)

Expand Down
24 changes: 23 additions & 1 deletion common/find_resource.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#include "drake/common/find_resource.h"

#include <cstdlib>
#include <filesystem>
#include <fstream>
#include <sstream>
#include <stdexcept>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -291,4 +293,24 @@ string FindResourceOrThrow(const string& resource_path) {
return FindResource(resource_path).get_absolute_path_or_throw();
}

std::optional<string> ReadFile(const std::filesystem::path& path) {
std::optional<string> result;
std::ifstream input(path, std::ios::binary);
if (input.is_open()) {
std::stringstream content;
content << input.rdbuf();
result.emplace(std::move(content).str());
}
return result;
}

std::string ReadFileOrThrow(const std::filesystem::path& path) {
std::optional<string> result = ReadFile(path);
if (!result) {
throw std::runtime_error(
fmt::format("Error reading from '{}'", path.string()));
}
return std::move(*result);
}

} // namespace drake
9 changes: 9 additions & 0 deletions common/find_resource.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <filesystem>
#include <optional>
#include <string>
#include <vector>
Expand Down Expand Up @@ -125,4 +126,12 @@ std::string FindResourceOrThrow(const std::string& resource_path);
/// copy of Drake, in case other methods have failed.
extern const char* const kDrakeResourceRootEnvironmentVariableName;

/// Returns the content of the file at the given path, or nullopt if it cannot
/// be read. Note that the path is a filesystem path, not a `resouce_path`.
std::optional<std::string> ReadFile(const std::filesystem::path& path);

/// Returns the content of the file at the given path, or throws if it cannot
/// be read. Note that the path is a filesystem path, not a `resouce_path`.
std::string ReadFileOrThrow(const std::filesystem::path& path);

} // namespace drake
18 changes: 9 additions & 9 deletions common/test/find_resource_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@

#include <cstdlib>
#include <filesystem>
#include <fstream>
#include <functional>
#include <memory>
#include <sstream>
#include <stdexcept>
#include <string>

Expand All @@ -16,6 +14,7 @@
#include "drake/common/drake_path.h"
#include "drake/common/drake_throw.h"
#include "drake/common/test_utilities/expect_no_throw.h"
#include "drake/common/test_utilities/expect_throws_message.h"

using std::string;

Expand Down Expand Up @@ -84,13 +83,8 @@ GTEST_TEST(FindResourceTest, FoundDeclaredData) {
// The path is the correct answer.
ASSERT_FALSE(absolute_path.empty());
EXPECT_EQ(absolute_path[0], '/');
std::ifstream input(absolute_path, std::ios::binary);
ASSERT_TRUE(input);
std::stringstream buffer;
buffer << input.rdbuf();
EXPECT_EQ(
buffer.str(),
"Test data for drake/common/test/find_resource_test.cc.\n");
EXPECT_EQ(ReadFileOrThrow(absolute_path),
"Test data for drake/common/test/find_resource_test.cc.\n");

// Sugar works the same way.
EXPECT_EQ(FindResourceOrThrow(relpath), absolute_path);
Expand Down Expand Up @@ -120,5 +114,11 @@ GTEST_TEST(GetDrakePathTest, PathIncludesDrake) {
EXPECT_TRUE(std::filesystem::exists(expected));
}

GTEST_TEST(ReadFileTest, NoSuchPath) {
EXPECT_FALSE(ReadFile("/foo/bar/missing"));
DRAKE_EXPECT_THROWS_MESSAGE(ReadFileOrThrow("/foo/bar/missing"),
"Error reading.*/foo/bar/missing.*");
}

} // namespace
} // namespace drake
12 changes: 1 addition & 11 deletions common/yaml/test/yaml_doxygen_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,6 @@ std::string WriteTemp(const std::string& data) {
return filename;
}

// Read data from a scratch file.
// (This is a test helper, not part of the Doxygen header.)
std::string ReadTemp(const std::string& filename) {
std::ifstream input(filename, std::ios::binary);
DRAKE_DEMAND(!input.fail());
std::stringstream buffer;
buffer << input.rdbuf();
return buffer.str();
}

// This is an example from the Doxygen header.
struct MyData {
template <typename Archive>
Expand Down Expand Up @@ -80,7 +70,7 @@ GTEST_TEST(YamlDoxygenTest, ExamplesSaving) {
SaveYamlFile(output_filename, data);

// This data is an example from the Doxygen header.
const std::string output = ReadTemp(output_filename);
const std::string output = ReadFileOrThrow(output_filename);
const std::string expected_output = R"""(
foo: 4.0
bar: [5.0, 6.0]
Expand Down
12 changes: 2 additions & 10 deletions common/yaml/test/yaml_io_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@ namespace yaml {
namespace test {
namespace {

std::string ReadTestFileAsString(const std::string& filename) {
std::ifstream input(filename, std::ios::binary);
DRAKE_DEMAND(!input.fail());
std::stringstream buffer;
buffer << input.rdbuf();
return buffer.str();
}

GTEST_TEST(YamlIoTest, LoadString) {
const std::string data = R"""(
value:
Expand Down Expand Up @@ -182,7 +174,7 @@ GTEST_TEST(YamlIoTest, SaveFile) {
const std::string filename = temp_directory() + "/SaveFile1.yaml";
const StringStruct data{.value = "save_file_1"};
SaveYamlFile(filename, data);
const auto result = ReadTestFileAsString(filename);
const auto result = ReadFileOrThrow(filename);
EXPECT_EQ(result, "value: save_file_1\n");
}

Expand All @@ -194,7 +186,7 @@ GTEST_TEST(YamlIoTest, SaveFileAllArgs) {
data.value.insert({"save_file_4", 1.0});
ASSERT_EQ(data.value.size(), 2);
SaveYamlFile(filename, data, child_name, {defaults});
const auto result = ReadTestFileAsString(filename);
const auto result = ReadFileOrThrow(filename);
EXPECT_EQ(result, "some_child:\n value:\n save_file_4: 1.0\n");
}

Expand Down
14 changes: 13 additions & 1 deletion geometry/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -452,13 +452,18 @@ drake_cc_googletest(

drake_cc_library(
name = "meshcat",
srcs = ["meshcat.cc"],
srcs = [
"meshcat.cc",
"meshcat_internal.cc",
],
hdrs = [
"meshcat.h",
"meshcat_internal.h",
"meshcat_types_internal.h",
],
data = [":meshcat_resources"],
install_hdrs_exclude = [
"meshcat_internal.h",
"meshcat_types_internal.h",
],
interface_deps = [
Expand Down Expand Up @@ -535,6 +540,13 @@ drake_cc_googletest(
],
)

drake_cc_googletest(
name = "meshcat_internal_test",
deps = [
":meshcat",
],
)

drake_cc_googletest(
name = "meshcat_denied_test",
allow_network = ["none"],
Expand Down
Loading

0 comments on commit 6a0d3bf

Please sign in to comment.