Skip to content

Add shared DMException for DM runtime errors - #2695

Merged
wixoaGit merged 5 commits into
OpenDreamProject:masterfrom
ahmdkaml:dmexception-shared
Aug 12, 2026
Merged

Add shared DMException for DM runtime errors#2695
wixoaGit merged 5 commits into
OpenDreamProject:masterfrom
ahmdkaml:dmexception-shared

Conversation

@ahmdkaml

Copy link
Copy Markdown
Contributor

Context

While investigating the use of generic Exception in the DM runtime, I found that this is substantially larger than a single-file change.

throw new Exception(...) is used across OpenDream for several different categories of failures: DM-facing runtime errors, runtime/VM invariants, resource handling, compiler/bytecode validation, and other internal errors. Determining which cases should become DMException therefore requires distinguishing DM-originated failures from OpenDream implementation failures.

I initially focused on DMOpcodeHandlers, where several cases were straightforward DM errors. I also verified this behavior with faulty DM programs: for example, invalid arithmetic operations and division by zero now produce OpenDreamShared.DMException while preserving the existing DM error message and behavior.

Current change

This PR introduces the shared DMException type and applies it to the confirmed DM-facing cases in DMOpcodeHandlers.

The intention here is not to change how these errors behave, but to give DM-originated runtime failures a distinct exception type.

Question about the intended design

Before expanding this conversion further, I'd like to confirm the intended architecture.

Once DMException exists, should it:

  1. Simply replace generic Exception at DM-facing failure sites while being handled by the existing runtime exception path?
  2. Have dedicated handling somewhere in the runtime, such as catching DMException and converting it into a DM-visible runtime error?
  3. Be handled differently from ordinary C# exceptions at a higher level?
  4. Follow another pattern already used elsewhere in OpenDream?

The distinction matters because there are many additional throw new Exception(...) sites, and some are clearly DM-facing while others appear to represent OpenDream/VM invariants. Converting everything mechanically could incorrectly classify internal failures as DM errors.

I'd therefore appreciate guidance on the intended handling and boundary for DMException before continuing with the remaining cases.

If the intended approach is confirmed, I think the remaining conversion would be better split into smaller, independently reviewable changes rather than making one very large cross-cutting PR.

Copilot AI lite review requested due to automatic review settings August 12, 2026 02:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@boring-cyborg boring-cyborg Bot added Client Involves the OpenDream client Runtime Involves the OpenDream server/runtime labels Aug 12, 2026
@ahmdkaml

Copy link
Copy Markdown
Contributor Author

The goal of this PR is not necessarily to be merged as-is, but to demonstrate and discuss a possible workflow for handling DM-originated exceptions.

The proposed workflow has three stages:

Create a shared DMException type so DM-originated errors have a distinct type from internal C# exceptions.
Replace the relevant throw new Exception(...) sites with throw new DMException(...), without changing the underlying behavior or error messages.
Add top-level exception handling that can distinguish between normal, debug, and other execution modes and decide what should be exposed to the user versus what should be logged/debugged.

This PR currently focuses on demonstrating the first two stages. I would like feedback on whether this is the intended direction, particularly on how DMException should be handled at the top level and what information should be shown in normal versus debug mode.

If this direction makes sense, the remaining conversion can be split into smaller, focused PRs rather than attempting to change the entire exception hierarchy in one large change.

@wixoaGit

Copy link
Copy Markdown
Member

The introduction of DMException is a good start here. I'm not sure how far up you mean by top-level exception handling though, and I wouldn't expect #2685's requested behavior to be as large a change as you say.

Any uncaught exception within executed DM code is handled by DreamThread's HandleException method. Here you can check if the exception is of type DMException, and exclude the C# stacktrace if the DEBUG compile flag isn't present.

@ahmdkaml

Copy link
Copy Markdown
Contributor Author

The introduction of DMException is a good start here. I'm not sure how far up you mean by top-level exception handling though, and I wouldn't expect #2685's requested behavior to be as large a change as you say.

Any uncaught exception within executed DM code is handled by DreamThread's HandleException method. Here you can check if the exception is of type DMException, and exclude the C# stacktrace if the DEBUG compile flag isn't present.

thanks for the quick response, I will start working u can expect the change soon.

@github-actions github-actions Bot added size/XL and removed size/L labels Aug 12, 2026
@ahmdkaml

Copy link
Copy Markdown
Contributor Author

Hey! I’ve pushed the dmexception-shared branch with the changes to use DMException for the DM-facing runtime errors. I also verified that the project builds successfully and all tests pass.

If there’s anything I missed or anything that needs to be adjusted, I’m happy to do any follow-ups.

@ahmdkaml

Copy link
Copy Markdown
Contributor Author

all the changes are strightforward dm exceptions I might have missed some but I didn't incorrectly include any

Comment thread OpenDreamShared/DMException.cs Outdated
Comment thread OpenDreamRuntime/Procs/NativeProc.cs Outdated
Comment thread OpenDreamRuntime/AtomManager.cs
Comment thread OpenDreamShared/DMException.cs Outdated
@wixoaGit wixoaGit linked an issue Aug 12, 2026 that may be closed by this pull request
Co-authored-by: wixoa <wixoag@gmail.com>
Comment thread OpenDreamRuntime/Objects/Types/DreamObjectWorld.cs Fixed
Comment thread OpenDreamRuntime/Procs/NativeProc.cs Fixed
Comment thread OpenDreamShared/DMException.cs Fixed
@ahmdkaml

Copy link
Copy Markdown
Contributor Author

all 4 changes requested are done. thanks for the quick review @wixoaGit

Comment thread OpenDreamRuntime/DMException.cs Fixed
Comment thread OpenDreamRuntime/DMException.cs Fixed
Comment thread OpenDreamRuntime/DMException.cs Outdated
Comment thread OpenDreamRuntime/DMException.cs Outdated
Co-authored-by: wixoa <wixoag@gmail.com>
@wixoaGit
wixoaGit enabled auto-merge (squash) August 12, 2026 18:42
@wixoaGit

Copy link
Copy Markdown
Member

The failing test is unrelated, I think #2677 made it flaky.

@wixoaGit
wixoaGit disabled auto-merge August 12, 2026 18:49
@wixoaGit
wixoaGit merged commit 60424fb into OpenDreamProject:master Aug 12, 2026
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client Involves the OpenDream client Runtime Involves the OpenDream server/runtime size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A new DMException error type

4 participants