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

ServerCallLogger incorrectly logs OK when call is canceled #370

Closed
bkeryan opened this issue Sep 8, 2023 · 2 comments · Fixed by #462
Closed

ServerCallLogger incorrectly logs OK when call is canceled #370

bkeryan opened this issue Sep 8, 2023 · 2 comments · Fixed by #462
Assignees
Labels
bug Something isn't working

Comments

@bkeryan
Copy link
Collaborator

bkeryan commented Sep 8, 2023

Bug Report

The server call logger logs OK when the call is canceled.

Repro or Code Sample

Run nifgen_standard_waveform and click the Stop button.

Expected Behavior

Log message says "CANCELLED".

Current Behavior

Log message says "OK":

PS D:\dev\measurementlink-python\examples\nifgen_standard_function> poetry run python .\measurement.py -v
2023-09-08 11:04:51,201 INFO: Measurement service hosted on port: 51284
2023-09-08 11:04:51,211 INFO: Successfully registered with discovery service.
Press enter to close the measurement service.
ed OK in 0.9954 ms
2023-09-08 11:04:56,595 INFO: Starting generation: pin_name=Pin1 waveform_type=Waveform.SINE frequency=1e+06 amplitude=2
2023-09-08 11:05:06,640 INFO: gRPC server call /ni.measurementlink.measurement.v2.MeasurementService/Measure responded OK in 10045.4617 ms
2023-09-08 11:05:09,758 INFO: Starting generation: pin_name=Pin1 waveform_type=Waveform.SINE frequency=1e+06 amplitude=2
2023-09-08 11:05:10,872 INFO: gRPC server call /ni.measurementlink.measurement.v2.MeasurementService/Measure responded OK in 1115.6107 ms
2023-09-08 11:05:28,375 INFO: Starting generation: pin_name=Pin1 waveform_type=Waveform.SINE frequency=1e+06 amplitude=2
2023-09-08 11:05:29,822 INFO: gRPC server call /ni.measurementlink.measurement.v2.MeasurementService/Measure responded OK in 1447.9786 ms

Possible Solution

Pass exception to close() method in logging code.

Your Environment

  • OS & Device: Windows
  • ni-measurementlink-service version: 1.2.0-dev2 from main
  • MeasurementLink version: 23.8.0d578
  • Python version: 3.9.13

AB#2517125

@bkeryan bkeryan added the bug Something isn't working label Sep 8, 2023
@bkeryan
Copy link
Collaborator Author

bkeryan commented Sep 8, 2023

This is partially fixed by #371

The part that isn't fixed: when the client calls measurement_service.context.abort(...), this always logs UNKNOWN regardless of the gRPC status code that the client specified. This happens because ServerContext.abort() raises an empty Exception rather than a RpcError containing the gRPC status code and details: https://github.com/grpc/grpc/blob/master/src/python/grpcio/grpc/_server.py#L392

@bkeryan bkeryan self-assigned this Sep 8, 2023
@bkeryan
Copy link
Collaborator Author

bkeryan commented Oct 12, 2023

Updating context.cancel() and context.abort() to log would help.

@vigkre vigkre linked a pull request Oct 18, 2023 that will close this issue
1 task
dixonjoel pushed a commit that referenced this issue Oct 19, 2023
* Added custom rpc error code in abort

* Fixed Lint errors

* Handled appropriate exception

* Removed unassigned variable

* Fixed spacing issues

* Refixed the Lint errors

* Handled log using code attribute

* Fixed Black issues

* Created Custom Rpc Error

* Fixed annotation errors

* Handled the exception explicitly

* Added return type for init

* exception handling using type

* Fixed exception
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant