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

Regress Puma to 3.10 to fix “no implicit conversion of nil into String” #504

Merged
merged 1 commit into from
Jan 22, 2018

Conversation

lgebhardt
Copy link
Contributor

Getting “no implicit conversion of nil into String” messages from puma. Looks like this is an issue introduced in 3.11 per puma/puma#1483, puma/puma#1502

I'm not entirely sure it's causing a real issue, but it is a nuisance. I propose we regress to 3.10 until this is fixed.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Tagged reviewers.

Test Plan:

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions

Security:

Temporary work around for “no implicit conversion of nil into String”
See puma/puma#1483 and puma/puma#1502
@mixonic
Copy link
Contributor

mixonic commented Jan 22, 2018

@lgebhardt I think this may be fixed by 3.11.2, via puma/puma@acb709b which was referenced on those tickets. I'll try it out.

@mixonic
Copy link
Contributor

mixonic commented Jan 22, 2018

So I expected this fix would speed up the server since I believe it is part of SSL negotiation failing. It indeed fixed performance issues with the server to either upgrade or downgrade 🎉 Good news.

v3.11.2 still logs an error Error reached top of thread-pool: EOFError (EOFError):

screen shot 2018-01-22 at 10 23 09 am

Thus I suggest shipping this regression of the version instead of the upgrade.

Copy link
Contributor

@ayumi ayumi left a comment

Choose a reason for hiding this comment

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

🍉

@ayumi ayumi merged commit 37b6054 into brave-intl:staging2 Jan 22, 2018
@lgebhardt lgebhardt deleted the puma310 branch January 26, 2018 19:37
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

3 participants