Improve DSM diagram loading error diagnostics (#286)#289
Conversation
When a .dsm file fails to load, the error messages were too terse to diagnose the root cause. This adds: - errno details on file open failures (e.g. "No such file or directory") - A hint when the file exists with a .dsm extension appended, suggesting the user fix their #include directive - Context showing which #include directive in which parent file caused a recursive load failure - The configured diag_path value in the top-level load error so users can verify their configuration
There was a problem hiding this comment.
Pull request overview
This PR improves error diagnostics when DSM diagram files fail to load. The changes add errno details on file open failures, a hint when users forget the .dsm extension in #include directives, context showing which #include directive caused recursive load failures, and the configured diag_path value in top-level load errors to help users verify their configuration.
Changes:
- Added errno-based error details using strerror() when file open fails
- Added a hint mechanism to detect and suggest when users forget .dsm extension in #include directives
- Added contextual error messages showing which #include directive in which file caused recursive load failures
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/dsm/DSMStateDiagramCollection.cpp | Added errno handling, .dsm extension hint detection, and contextual error messages for #include failures |
| apps/dsm/DSM.cpp | Enhanced top-level diagram load error message with diag_path configuration hint |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ERROR("loading state diagram '%s': cannot open file (%s)\n", | ||
| filename.c_str(), strerror(errno)); | ||
|
|
||
| // Check if the user forgot the .dsm extension (common #include mistake) | ||
| string with_ext = filename + ".dsm"; | ||
| ifstream test_ifs(with_ext.c_str()); | ||
| if (test_ifs.good()) { | ||
| ERROR(" Hint: '%s' exists - did you mean to use '%s' in your #include directive?\n", | ||
| with_ext.c_str(), with_ext.c_str()); |
There was a problem hiding this comment.
The errno value may not be reliable after an ifstream constructor or good() call fails. C++ iostream operations do not guarantee that errno is set on failure. The strerror(errno) call may report a stale or incorrect error code. Consider using failbit and checking specific failure modes, or save errno immediately after operations that are guaranteed to set it (like open() syscalls), rather than relying on errno after iostream operations.
| ERROR("loading state diagram '%s': cannot open file (%s)\n", | |
| filename.c_str(), strerror(errno)); | |
| // Check if the user forgot the .dsm extension (common #include mistake) | |
| string with_ext = filename + ".dsm"; | |
| ifstream test_ifs(with_ext.c_str()); | |
| if (test_ifs.good()) { | |
| ERROR(" Hint: '%s' exists - did you mean to use '%s' in your #include directive?\n", | |
| with_ext.c_str(), with_ext.c_str()); | |
| ERROR("loading state diagram '%s': cannot open file\n", | |
| filename.c_str()); | |
| // Check if the user forgot the .dsm extension (common #include mistake) | |
| string with_ext = filename + ".dsm"; | |
| ifstream test_ifs(with_ext.c_str()); | |
| if (test_ifs.good()) { | |
| ERROR(" Hint: '%s' exists - did you mean to use '%s' in your #include directive?\n", | |
| with_ext.c_str(), with_ext.c_str()); |
When a .dsm file fails to load, the error messages were too terse to diagnose the root cause. This adds: