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

fix: Don't override Ruby stdout with stderr #1336

Merged
merged 1 commit into from
Mar 8, 2022
Merged

fix: Don't override Ruby stdout with stderr #1336

merged 1 commit into from
Mar 8, 2022

Conversation

shalvah
Copy link
Contributor

@shalvah shalvah commented Feb 13, 2022

Description

This PR prevents from stdout from being replaced with stderr in responses from Ruby handlers.

Motivation and Context

When I run my Ruby handler (which uses serverless-rack to run a Sinatra application, I noticed that whenever I add a logging statement (such as puts or logger.info), the response from my handler no longer shows in my browser. Instead, I get the logs in my browser (in addition to the console).

Same thing happens when there's an uncaught error. My app's error page is bypassed, and instead my error logs are printed to the browser.

I found out that this is because Rack's error stream is stderr (as it should be), and these lines in this project:

if (stderr) {
// TODO
if (this.log) {
this.log.notice(stderr)
} else {
console.log(stderr)
}
return stderr
}

return stderr as the response when it is non-empty. But there is no good reason for this. The framework's own guide says plugins should write logs (not just errors) to stderr and output to stdout, so it makes no sense to sometimes return the logs instead of the output. Also, the other runners don't do this.

How Has This Been Tested?

I pointed my local app to this change, and it worked as expected. My response is always returned, and logs remain on the console only.

@shalvah
Copy link
Contributor Author

shalvah commented Mar 7, 2022

@pgrzesik any chance of getting this merged? It's a one-liner.🙏

Copy link
Collaborator

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Thanks @shalvah 🙇

@pgrzesik pgrzesik changed the title Fix: Don't override Ruby stdout with stderr fix: Don't override Ruby stdout with stderr Mar 8, 2022
@pgrzesik pgrzesik merged commit 353b6c6 into dherault:master Mar 8, 2022
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.

None yet

2 participants