Sunshine icon indicating copy to clipboard operation
Sunshine copied to clipboard

fix(launch): Fix several launch failure conditions (exceptions thrown in child.wait, and boost::split_unix)

Open mcourteaux opened this issue 1 month ago • 8 comments

Description

  • fix(launch): Boost split_unix/split_winmain can throw exceptions, which disrupted control flow. Surrounded with try-catch, reporting a helpful message.
    • For me, split_unix was throwing on \! as escape sequence. The \ got added by saving/loading the json file I think, not sure.
  • improve(launch): In case unexpected exceptions are thrown during launch or resume, an empty an error is now logged to help the user analyze this.
  • fix(process): child.wait() may throw, replaced it with the std::error_code variant.

This fixes the Malformed XML error moonlight-qt showed in my situation.

Screenshot

Issues Fixed or Closed

Resolves #4038.

Roadmap Issues

Type of Change

  • [ ] feat: New feature (non-breaking change which adds functionality)
  • [x] fix: Bug fix (non-breaking change which fixes an issue)
  • [ ] docs: Documentation only changes
  • [ ] style: Changes that do not affect the meaning of the code (white-space, formatting, missing semicolons, etc.)
  • [ ] refactor: Code change that neither fixes a bug nor adds a feature
  • [ ] perf: Code change that improves performance
  • [ ] test: Adding missing tests or correcting existing tests
  • [ ] build: Changes that affect the build system or external dependencies
  • [ ] ci: Changes to CI configuration files and scripts
  • [ ] chore: Other changes that don't modify src or test files
  • [ ] revert: Reverts a previous commit
  • [ ] BREAKING CHANGE: Introduces a breaking change (can be combined with any type above)

Checklist

  • [x] Code follows the style guidelines of this project
  • [x] Code has been self-reviewed
  • [ ] Code has been commented, particularly in hard-to-understand areas
  • [ ] Code docstring/documentation-blocks for new or existing methods/components have been added or updated
  • [ ] Unit tests have been added or updated for any new or modified functionality

AI Usage

  • [x] None: No AI tools were used in creating this PR
  • [ ] Light: AI provided minor assistance (formatting, simple suggestions)
  • [ ] Moderate: AI helped with code generation or debugging specific parts
  • [ ] Heavy: AI generated most or all of the code changes

mcourteaux avatar Nov 05 '25 13:11 mcourteaux

Bundle Report

Bundle size has no change :white_check_mark:

codecov[bot] avatar Nov 05 '25 13:11 codecov[bot]

Codecov Report

:x: Patch coverage is 0% with 17 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 14.19%. Comparing base (1d6d916) to head (238312d). :warning: Report is 1 commits behind head on master. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/process.cpp 0.00% 9 Missing and 4 partials :warning:
src/nvhttp.cpp 0.00% 2 Missing and 2 partials :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4390      +/-   ##
==========================================
- Coverage   14.94%   14.19%   -0.76%     
==========================================
  Files          63       89      +26     
  Lines       13726    18936    +5210     
  Branches     6476     8677    +2201     
==========================================
+ Hits         2052     2688     +636     
- Misses       8686    12969    +4283     
- Partials     2988     3279     +291     
Flag Coverage Δ
FreeBSD-14.3-aarch64 ?
FreeBSD-14.3-amd64 13.52% <0.00%> (-0.03%) :arrow_down:
Linux-AppImage 11.55% <0.00%> (-0.02%) :arrow_down:
Windows-AMD64 13.34% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/nvhttp.cpp 18.35% <0.00%> (+0.45%) :arrow_up:
src/process.cpp 1.03% <0.00%> (-0.05%) :arrow_down:

... and 52 files with indirect coverage changes

codecov[bot] avatar Nov 05 '25 14:11 codecov[bot]

Can somebody update me on this? Should anything happen before merge?

mcourteaux avatar Nov 10 '25 08:11 mcourteaux

@mcourteaux Thank you for the PR!

There is an error while compiling on Windows which needs to be fixed.

The error in ArchLinux can be ignored, it's a known issue.

Could also rebase when you push your fixes?

ReenigneArcher avatar Nov 10 '25 13:11 ReenigneArcher

There is an error while compiling on Windows which needs to be fixed.

Did that I believe.

Could also rebase when you push your fixes?

Done.

mcourteaux avatar Nov 12 '25 00:11 mcourteaux

Fixed the clang-format issue, and rebased again.

mcourteaux avatar Nov 12 '25 00:11 mcourteaux

Fixed the clang-format issue, and rebased again.

Thanks! Looks like you mistakenly changed the build-deps submodule.

ReenigneArcher avatar Nov 12 '25 01:11 ReenigneArcher

TIL: while checked out on a feature-branch, doing git fetch origin master:master does not update the submodules.

Looks like you mistakenly changed the build-deps submodule.

Should be fixed now.

mcourteaux avatar Nov 12 '25 08:11 mcourteaux