microsoft / microsoft/terminal

Corrupted last error after deprecated/unsupported API calls

Open
#14,479 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Area-Server Help Wanted Issue-Bug Priority-3 Product-Conhost
Dominant language
C++
Stars
105k
Forks
9.6k
Avg merge
3d 17h
Merged PRs (30d)
29

Description

### Windows Terminal version

_No response_

### Windows build number

10.0.19044.2006

### Other Software

_No response_

### Steps to reproduce

Basically the problem is nicely described in [this SO answer](https://stackoverflow.com/a/61475022).

https://github.com/microsoft/terminal/blob/3c78e01ab5a025290b8953a6dc0f2695c76c223e/src/server/ApiDispatchersInternal.cpp#L22
`ServerDeprecatedApi` uses `E_NOTIMPL` (0x80004001), which on its way to the caller suffers several transformations and becomes 0x00004001, which is not a valid Windows error code.

It is quite unfortunate, given that the whole idea is to make it clear to the caller that the call is deprecated/not implemented.

Perhaps the function could use a different code that won't get fubared beyond recognition?
Given that it is passed to `RtlNtStatusToDosError` on its way, it probably should be `NTSTATUS` in the first place, either just raw `STATUS_NOT_IMPLEMENTED` or packed into HRESULT as `HRESULT_FROM_NT(STATUS_NOT_IMPLEMENTED)`.

### Expected Behavior

After an unsupported call:
- `GetLastError()` should return a sensible value, e.g. `ERROR_INVALID_FUNCTION` or `ERROR_CALL_NOT_IMPLEMENTED`
- (optionally) `RtlGetLastNtStatus()` should return a sensible value, e.g. `STATUS_NOT_IMPLEMENTED`

### Actual Behavior

- `GetLastError()` returns 0x0004001, which is an invalid error code.
- `RtlGetLastNtStatus()` returns 0xC0074001, which is the same invalid win32 error code with `STATUS_SEVERITY_ERROR` and `FACILITY_NTWIN32` bits.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/server/ApiDispatchersInternal.cpp at ServerDeprecatedApi and trace how its return value reaches GetLastError() and RtlGetLastNtStatus(). Compare the conversion path with the expected Windows error behavior described in the issue. Done means unsupported or deprecated calls produce a valid not-implemented error instead of 0x00004001.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.