Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Lua module methods to never return errors #150

Closed
wants to merge 1 commit into from
Closed

Conversation

rkusa
Copy link
Collaborator

@rkusa rkusa commented May 23, 2022

We had issues with a crash from an error returned by the dcs_grpc.dll in the past, see #19. We fixed that by directly logging the offending error instead of returning it to Lua, see bd54607.

I've investigated this again. I tried to reproduce this issue outside of DCS (basically with a lua.exe and a minimal Lua module written in Rust using mlua), but was unsuccessful - I couldn't reproduce it outside of DCS. As I don't have any idea left of how to fix/narrow down this issue, I give up and and would suggest to avoid returning errors from Rust to Lua entirely. This is also what this PR changes. It ensures that all calls to grpc.* methods never error. If they encounter an error internally, the error is logged right away and not returned to Lua first.

Drawback: The errors will not land in the Logs/dcs.log file and instead only in Logs/gRPC.log

Potentially fixes the crash reported by the Enigma server admins (see https://discord.com/channels/696721363276267590/962651858760003594/977730972206366781).
Fixes #138

@rkusa rkusa marked this pull request as ready for review May 23, 2022 16:18
@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `GetTheatre` API
- `GetUnitType` API

### Changed
- Improved reliability of error handling.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick. Could we expand on this where we mention that all errors are now logged only to grpc.log

Copy link
Contributor

Choose a reason for hiding this comment

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

Just saw in related comment that this only happens in hot-loading. In that case I am fine to continue returning errors to lua and maybe we just document this as a known issue if you are using hot_loading (which no production server should be using as far as I am concerned).

@rkusa
Copy link
Collaborator Author

rkusa commented Jun 5, 2022

On hold for now, see #138 (comment)

@rkusa rkusa marked this pull request as draft June 5, 2022 07:48
@rurounijones
Copy link
Contributor

Closing as no longer needed.

@rurounijones rurounijones deleted the errors branch August 14, 2022 02:25
@rkusa
Copy link
Collaborator Author

rkusa commented Sep 9, 2022

For the record, we closed this PR as we've only ever reproduced it while using the development hot-reload functionality and figured, as it doesn't happen for normal installations, that for now, it wasn't worth the drawback of the error logs only appearing in Logs/gRPC.log.

@rkusa
Copy link
Collaborator Author

rkusa commented Oct 5, 2022

Fixed with #188

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't return errors from Rust to Lua
2 participants