Skip to content

fix: Handle stderr properly in SSH transport #75

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

Closed
wants to merge 1 commit into from

Conversation

kadler
Copy link
Member

@kadler kadler commented Sep 6, 2022

Instead of reading a small amount of data in a blocking way from the
stdout and stderr streams, enable non-blocking I/O and attempt to read
as much data as possible. In addition, make a convenience function to
handle the stream being closed and timeouts so that we don't need to
store a temp variable in the loop. This prevents the situation that lead
to issue #74 where the stderr data is written to the output XML if
stdout is already closed.

Fixes #74

@kadler kadler requested review from ThePrez and abmusse September 6, 2022 16:56
Instead of reading a small amount of data in a blocking way from the
stdout and stderr streams, enable non-blocking I/O and attempt to read
as much data as possible. In addition, make a convenience function to
handle the stream being closed and timeouts so that we don't need to
store a temp variable in the loop. This prevents the situation that lead
to issue #74 where the stderr data is written to the output XML if
stdout is already closed.

Fixes #74
@kadler kadler force-pushed the ssh-transport-stderr branch from a555584 to 17460d7 Compare September 6, 2022 18:43
@NattyNarwhal
Copy link

this looks right? before

***********************
control Fri Feb  3 15:56:19 2023
 ipc(*na) ctl(*here *cdata)
input Fri Feb  3 15:56:19 2023
<?xml version='1.0'?>
<xmlservice><sh error="fast" var="wrkactjob"><![CDATA[/QOpenSys/usr/bin/system WRKACTJOB]]></sh></xmlservice>

output Fri Feb  3 15:56:20 2023
b' directory\n'
parse (fail) Fri Feb  3 15:56:20 2023
parse step: 3 (1-ok, 2-*BADPARSE, 3-*NOPARSE)
{'error': {'error': '*NOPARSE', 'wrkactjob': {...}}}

after

***********************
control Fri Feb  3 15:59:39 2023
 ipc(*na) ctl(*here *cdata)
input Fri Feb  3 15:59:39 2023
<?xml version="1.0" ?><xmlservice><sh var="wrkactjob" error="fast"><![CDATA[/QOpenSys/usr/bin/system WRKACTJOB]]></sh></xmlservice>
output Fri Feb  3 15:59:40 2023
<?xml version='1.0'?>
<xmlservice>
<error>*NODATA</error>
</xmlservice>
parse step: 1 (1-ok, 2-*BADPARSE, 3-*NOPARSE)

@kadler
Copy link
Member Author

kadler commented Feb 3, 2023

Merged in e16b965

@kadler kadler closed this Feb 3, 2023
@kadler kadler deleted the ssh-transport-stderr branch February 3, 2023 21:16
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.

Stderr handling in SSH Transport can yield wrong results
2 participants