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

Method writeBytes conceals error details #96

Closed
hiddenalpha opened this issue Jul 29, 2021 · 2 comments
Closed

Method writeBytes conceals error details #96

hiddenalpha opened this issue Jul 29, 2021 · 2 comments
Milestone

Comments

@hiddenalpha
Copy link

hiddenalpha commented Jul 29, 2021

Actual Behavior

Method

Java_jssc_SerialNativeInterface_writeBytes(JNIEnv*, jobject, jlong, jbyteArray);

Conceals error details.

_writeBytes just returns true/false on success/failure without any further details. But write() would provide (potentially useful) error details (For example see a list at its man page).

Further, SerialPort.writeBytes just communicates errors by returning false. This has potential to fool java developers. As they're used to get exceptions thrown if someting is wrong. Java code, checking for returned success/failure codes is something I see seldom. I even found exactly this mistake in our projects codebase which is using jssc. This is not meant to blame java devs. It's just when writing java, one easily overlooks this as one is used to exceptions and does not expect a method where the return value suddenly should be checked (in contrast of all other methods around id).

Expected Behavior

IMHO we should provide more error details to our caller. I see multiple ways to do so. Eg we could throw java exceptions (for example from SerialPort class but also from native code would be possible). Or returning numerical codes, Or providing some kind of getError() method (not very nice, but better than nothing) alongside with the returned boolean where caller can retrieve an error message. Or ... whatever.

hiddenalpha added a commit to hiddenalpha/jssc that referenced this issue Oct 3, 2022
Some thoughts about
java-native#129
and
java-native#96

What this commit does:

- Make native impls return numberOfBytesWritten.
- Throw SerialPortException in case write fails in some way.
- Expose new 'write()' method to java which returns
  numberOfBytesWritten and uses exceptions for error handling.
- Keep old 'writeBytes()' method and translate to the old behavior.

What this commit does NOT:

THIS CODE IS NOT TESTED! It even was not launched at all. Nevertheless
it did compile fine for x86_64-linux-gnu (debian) and mingw
(cross-compile). So we need to do:

- Testing!
- Review if the change in the API (adding a 'write()' method) is the
  thing we should do or if we should do it differently somehow.

(wip @ 36ee5d8)
@hiddenalpha
Copy link
Author

hiddenalpha commented Apr 26, 2023

Relates to #136 and #138 (in perspective of missing errorhandling)

@hiddenalpha
Copy link
Author

Fixed through #146 and #152.

@pietrygamat pietrygamat added this to the 2.9.6 milestone Dec 15, 2023
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

No branches or pull requests

2 participants