Skip to content

retro_load always reports "loaded" even when underlying load fails #123

Description

@c64cryptoboy

Describe the bug
retro_load currently appears to return success even when the requested file was not actually loaded/mounted.

The MCP tool always reports "loaded" even if the lower-level load endpoint failed, for example because the file path is not valid from the Retro Debugger process’s point of view.

To Reproduce
Steps to reproduce the behavior:

  1. Run Retro Debugger as a Windows process with MCP enabled.

  2. Call retro_load with a path that exists in the environment but not as a valid Windows path for Retro Debugger, e.g.:

  /docker-share/c64TestData/test.d64
  1. Observe that retro_load returns:
  {
	"status": "loaded",
	"path": "/docker-share/c64TestData/test.d64"
  }
  1. From the emulated C64 inside Retro Debugger, query the drive command channel.

  2. Observe drive error 21, indicating the mount failed / drive image is not usable.

  3. Reformatting the path as a Windows-visible path succeeds:

  C:\docker-share\c64TestData\test.d64

The C64 drive status then reports 0 / no error. Note that the C: drive letter was required.

Expected behavior
retro_load should not unconditionally return "loaded".

If the underlying load endpoint fails, for example because SYS_FileExists(fileName.c_str()) fails, then retro_load should return an MCP error or at least a non-success status.

Screenshots
Not applicable.

Desktop (please complete the following information):

  • OS: Windows 11

Additional context
Likely cause for reporting success on failure:

In src/Remote/MCP/CMCPServer.cpp, the retro_load handler calls:

vector<char> *result = server->RunEndpointFunction("load", "", ep, nullptr, 0);
delete result;
return {{"status", "loaded"}, {"path", params.at("path")}};

The result from the underlying load endpoint is discarded, so MCP always reports "loaded" regardless of whether the endpoint returned an error such as file-not-found.

The lower-level load endpoint in CDebuggerServerWebSockets.cpp does check:

if (!SYS_FileExists(fileName.c_str()))

and can return HTTP_NOT_FOUND, but that failure is not propagated by retro_load.

Suggested fix:

Have retro_load parse/propagate the underlying endpoint result/status instead of unconditionally returning "loaded". If the file does not exist from the Retro Debugger process’s point of view, retro_load should return an error.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions