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

Regression: Trilogy::SyscallError::EADDRNOTAVAIL no longer converted to ActiveRecord::StatementInvalid #52

Closed
dhruvCW opened this issue Jul 10, 2023 · 10 comments

Comments

@dhruvCW
Copy link
Contributor

dhruvCW commented Jul 10, 2023

In our CI when we run

#!/bin/bash -eo pipefail
if (egrep '^  grape($|\s)' Gemfile.lock > /dev/null) ; then
  bundle exec rails runner 'puts API::Base.routes.map(&:path)' > grape-routes.txt
fi

we get the following error message (since trilogy adapter 3.1.0)

 /home/circleci/project/vendor/bundle/ruby/3.2.0/gems/trilogy-2.4.1/lib/trilogy.rb:174:in `_initialize': Cannot assign requested address - trilogy_connect - unable to connect to localhost:3306 (Trilogy::SyscallError::EADDRNOTAVAIL)

this is a regression since previously it used to result in

Failed to validate the schema cache because of ActiveRecord::StatementInvalid: Trilogy::SyscallError::EADDRNOTAVAIL: Cannot assign requested address - trilogy_connect - unable to connect to localhost:3306
Failed to validate the schema cache because of ActiveRecord::StatementInvalid: Trilogy::SyscallError::EADDRNOTAVAIL: Cannot assign requested address - trilogy_connect - unable to connect to localhost:3306

without any breaks.

@dhruvCW dhruvCW changed the title rails breaks due to unhandled Trilogy::SyscallError::EADDRNOTAVAIL Regression: Trilogy::SyscallError::EADDRNOTAVAIL no longer converted to ActiveRecord::StatementInvalid Jul 10, 2023
@dhruvCW
Copy link
Contributor Author

dhruvCW commented Jul 10, 2023

fixed in trilogy-libraries/trilogy#98

@dbussink
Copy link
Contributor

What does this behave like on Rails main itself with Trilogy? #49 removed the translation since that broke other things and matches the upstream Rails code more closely. Does Rails main fail with the same error as you show here as well?

@dhruvCW
Copy link
Contributor Author

dhruvCW commented Jul 10, 2023

Haven't tested this on rails main but this isn't actually a problem the adapter should solve. The client should be returning this as a Trilogy::BaseConnectionError which is then translated by the adapter as a ActiveRecord::DatabaseConnectionError. I believe #49 just exposed this problem.

with trilogy-libraries/trilogy#98 the behaviour is now the same as with mysql2.

@lorint
Copy link
Collaborator

lorint commented Jul 11, 2023

@dhruvCW, believe I'd now like to know more about this perfect situation.

@dhruvCW
Copy link
Contributor Author

dhruvCW commented Jul 11, 2023

@dhruvCW, believe I'd now like to know more about this perfect situation.

If you are refering to the situation that triggers this error. It seems to exclusively affect our usage of circle-ci. We have two parallel jobs one that runs rspec against a test database as a CI dependency (splitting the tests across multiple jobs), and a job that run rspec + the above mentioned route dumps. The latter is the point where we face this problem. Based on a bit of google-fu it seems that the client cannot connect to the port/host combination (localhost) because another entity is already bound to it. This might make sense if we have multiple processes accessing the port but I am not sure my reasoning is right 🤷

as for the solution, handling this as a Trilogy::BaseConnectionError with the text DNS_ERROR in the client initialization code allows us (the adapter) to handle this as a DBConnectionError for the hostname which IMO is the correct solution since that is also the result in this situation with mysql2.

@lorint
Copy link
Collaborator

lorint commented Jul 11, 2023

the client cannot connect to the port/host combination (localhost) because another entity is already bound to it

Seems curious only because MySQL should allow for 150 or 200 connections. You can see how many your instance has set by running:

mysql -uroot --execute='SHOW VARIABLES LIKE "max_connections";' 

@dhruvCW
Copy link
Contributor Author

dhruvCW commented Jul 11, 2023

Seems curious only because MySQL should allow for 150 or 200 connections. You can see how many your instance has set by running:
mysql -uroot --execute='SHOW VARIABLES LIKE "max_connections";'

my apologies for not being clear we don't have a running mysql server for the job that fails (the socket error seems to be for the client itself).

@lorint
Copy link
Collaborator

lorint commented Jul 11, 2023

Ah, cool.

As a side-note, there is a PR for edge Rails that is likely to drop soon which affects Trilogy error messages. It does not look like it will impact this issue, but I wanted to examine this as we move forward with a fix.

@dhruvCW
Copy link
Contributor Author

dhruvCW commented Jul 11, 2023

As a side-note, there is a rails/rails#48109. It does not look like it will impact this issue, but I wanted to examine this as we move forward with a fix.

thanks for sharing this PR based on the proposed changes this situation should result in a ConnectionFailed error (since on trilogy main all exceptions are derived from Trilogy::Error). so we should be fine 👍

@dhruvCW
Copy link
Contributor Author

dhruvCW commented Aug 23, 2023

closing in favour of trilogy-libraries/trilogy#113

@dhruvCW dhruvCW closed this as completed Aug 23, 2023
@dhruvCW dhruvCW closed this as not planned Won't fix, can't repro, duplicate, stale Aug 23, 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

Successfully merging a pull request may close this issue.

3 participants