Skip to content

Error message on failed write #8822

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.

Sign up for GitHub

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

Open
fliiiix wants to merge 5 commits into
base: master
Choose a base branch
from
+113 −65
Open

Error message on failed write #8822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9058c59
[Python] Better error messages on write
fliiiix Dec 3, 2025
6544ef9
[Python] Apply clang-format
fliiiix Dec 3, 2025
9df6150
[Python] Convert GRPC to const char* return
fliiiix Dec 3, 2025
8b427fe
[Python] Apply clang-format
fliiiix Dec 3, 2025
5d5568e
Merge branch 'master' into feature/error-message-on-failed-write
aardappel Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions grpc/src/compiler/python_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class StubGenerator : public BaseGenerator {
const Version& version)
: BaseGenerator(parser, kStubConfig, path, version) {}

bool Generate() {
const char* Generate() {
Imports imports;
std::stringstream stub;
std::string ns_name{};
Expand All @@ -150,8 +150,8 @@ class StubGenerator : public BaseGenerator {
}

private:
bool SaveStub(const std::string& filename, const Imports& imports,
const std::string& content) {
const char* SaveStub(const std::string& filename, const Imports& imports,
const std::string& content) {
std::stringstream ss;
ss << "# Generated by the gRPC FlatBuffers compiler. DO NOT EDIT!\n"
<< '\n'
Expand All @@ -161,7 +161,8 @@ class StubGenerator : public BaseGenerator {
ss << content << '\n';

EnsureDirExists(StripFileName(filename));
return parser_.opts.file_saver->SaveFile(filename.c_str(), ss.str(), false);
return parser_.opts.file_saver->AttemptSave(filename.c_str(), ss.str(),
false);
}

void Generate(std::stringstream& ss, const ServiceDef* service,
Expand Down Expand Up @@ -246,7 +247,7 @@ class ServiceGenerator : public BaseGenerator {
const Version& version)
: BaseGenerator(parser, kConfig, path, version) {}

bool Generate() {
const char* Generate() {
Imports imports;
std::stringstream ss;

Expand Down Expand Up @@ -282,16 +283,17 @@ class ServiceGenerator : public BaseGenerator {
}

private:
bool SaveService(const std::string& filename, const Imports& imports,
const std::string& content) {
const char* SaveService(const std::string& filename, const Imports& imports,
const std::string& content) {
std::stringstream ss;
ss << "# Generated by the gRPC FlatBuffers compiler. DO NOT EDIT!\n"
<< '\n';
FormatImports(ss, imports);
ss << content << '\n';

EnsureDirExists(StripFileName(filename));
return parser_.opts.file_saver->SaveFile(filename.c_str(), ss.str(), false);
return parser_.opts.file_saver->AttemptSave(filename.c_str(), ss.str(),
false);
}

void GenerateStub(std::stringstream& ss, const ServiceDef* service,
Expand Down Expand Up @@ -395,14 +397,14 @@ class ServiceGenerator : public BaseGenerator {
};
} // namespace

bool Generate(const Parser& parser, const std::string& path,
const Version& version) {
const char* Generate(const Parser& parser, const std::string& path,
const Version& version) {
ServiceGenerator generator{parser, path, version};
return generator.Generate();
}

bool GenerateStub(const Parser& parser, const std::string& path,
const Version& version) {
const char* GenerateStub(const Parser& parser, const std::string& path,
const Version& version) {
StubGenerator generator{parser, path, version};
return generator.Generate();
}
Expand Down
8 changes: 4 additions & 4 deletions grpc/src/compiler/python_generator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@
namespace flatbuffers {
namespace python {
namespace grpc {
bool Generate(const Parser& parser, const std::string& path,
const Version& version);
const char* Generate(const Parser& parser, const std::string& path,
const Version& version);

bool GenerateStub(const Parser& parser, const std::string& path,
const Version& version);
const char* GenerateStub(const Parser& parser, const std::string& path,
const Version& version);
} // namespace grpc
} // namespace python
} // namespace flatbuffers
Expand Down
4 changes: 4 additions & 0 deletions include/flatbuffers/code_generators.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ class BaseGenerator {
public:
virtual bool generate() = 0;

// Modern alternative to bool generate() as this provides a way to
// return a specific error message in case of an error and nullptr on success.
virtual const char* Generate() { return nullptr; }

static std::string NamespaceDir(const Parser& parser, const std::string& path,
const Namespace& ns,
const bool dasherize = false);
Expand Down
14 changes: 14 additions & 0 deletions include/flatbuffers/file_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,18 @@ class FileSaver {
virtual bool SaveFile(const char* name, const char* buf, size_t len,
bool binary) = 0;

virtual const char* AttemptSave(const char* name, const char* buf, size_t len,
bool binary) = 0;

bool SaveFile(const char* name, const std::string& buf, bool binary) {
return SaveFile(name, buf.c_str(), buf.size(), binary);
}

const char* AttemptSave(const char* name, const std::string& buf,
bool binary) {
return AttemptSave(name, buf.c_str(), buf.size(), binary);
}

virtual void Finish() {}

private:
Expand All @@ -52,13 +60,19 @@ class RealFileSaver final : public FileSaver {
public:
bool SaveFile(const char* name, const char* buf, size_t len,
bool binary) final;

const char* AttemptSave(const char* name, const char* buf, size_t len,
bool binary) final;
};

class FileNameSaver final : public FileSaver {
public:
bool SaveFile(const char* name, const char* buf, size_t len,
bool binary) final;

const char* AttemptSave(const char* name, const char* buf, size_t len,
bool binary) final;

void Finish() final;

private:
Expand Down
4 changes: 2 additions & 2 deletions include/flatbuffers/idl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1303,8 +1303,8 @@ bool GenerateJavaGRPC(const Parser& parser, const std::string& path,

// Generate GRPC Python interfaces.
// See idl_gen_grpc.cpp.
bool GeneratePythonGRPC(const Parser& parser, const std::string& path,
const std::string& file_name);
const char* GeneratePythonGRPC(const Parser& parser, const std::string& path,
const std::string& file_name);

// Generate GRPC Swift interfaces.
// See idl_gen_grpc.cpp.
Expand Down
13 changes: 11 additions & 2 deletions src/file_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
* limitations under the License.
*/

#include "flatbuffers/file_manager.h"

#include <fstream>
#include <set>
#include <string>

#include "flatbuffers/file_manager.h"

namespace flatbuffers {

bool RealFileSaver::SaveFile(const char* name, const char* buf, size_t len,
Expand All @@ -30,4 +30,13 @@ bool RealFileSaver::SaveFile(const char* name, const char* buf, size_t len,
return !ofs.bad();
}

const char* RealFileSaver::AttemptSave(const char* name, const char* buf,
size_t len, bool binary) {
if (!SaveFile(name, buf, len, binary)) {
static std::string error_msg;
Copy link
Collaborator

@aardappel aardappel Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can have static std::string in this project. This will make the project non-modular in terms of memory management. Better to stick an err string in RealFileSaver ? Or, if we absolutely must do string formatting for these errors, probably the least bad would be to return a std::string by value from this function, where empty string means no error. But really, you are formatting name which is passed in by the called, so they already know the cause of the problem, so a non-formatted const char * may be better.

Copy link
Contributor Author

@fliiiix fliiiix Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair point i will update 👍

error_msg = "Could not save file " + std::string(name ? name : "unknown");
return error_msg.c_str();
}
return nullptr;
}
} // namespace flatbuffers
6 changes: 6 additions & 0 deletions src/file_name_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ bool FileNameSaver::SaveFile(const char* name, const char* buf, size_t len,
return true;
}

const char* FileNameSaver::AttemptSave(const char* name, const char* buf,
size_t len, bool binary) {
return SaveFile(name, buf, len, binary) ? nullptr
: "Printing filename failed";
}

void FileNameSaver::Finish() {
for (const auto& file_name : file_names_) {
// Just print the file names to standard output.
Expand Down
19 changes: 10 additions & 9 deletions src/idl_gen_grpc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,25 +440,26 @@ bool GenerateJavaGRPC(const Parser& parser, const std::string& path,
return JavaGRPCGenerator(parser, path, file_name).generate();
}

bool GeneratePythonGRPC(const Parser& parser, const std::string& path,
const std::string& /*file_name*/) {
const char* GeneratePythonGRPC(const Parser& parser, const std::string& path,
const std::string& /*file_name*/) {
int nservices = 0;
for (auto it = parser.services_.vec.begin(); it != parser.services_.vec.end();
++it) {
if (!(*it)->generated) nservices++;
}
if (!nservices) return true;
if (!nservices) return nullptr;

flatbuffers::python::Version version{parser.opts.python_version};
if (!version.IsValid()) return false;
if (!version.IsValid()) return "The provided Python version is not valid";

auto error = flatbuffers::python::grpc::Generate(parser, path, version);
if (error) return error;

if (!flatbuffers::python::grpc::Generate(parser, path, version)) {
return false;
}
if (parser.opts.python_typing) {
return flatbuffers::python::grpc::GenerateStub(parser, path, version);
auto error = flatbuffers::python::grpc::GenerateStub(parser, path, version);
if (error) return error;
}
return true;
return nullptr;
}

class SwiftGRPCGenerator : public flatbuffers::BaseGenerator {
Expand Down
Loading
Loading