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

catching `LoadError` when loading `database.rb` leads to confusing errors #2125

Closed
fnordfish opened this Issue Mar 10, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@fnordfish
Contributor

fnordfish commented Mar 10, 2017

I've run into an interesting to track down error using padrino 0.13.3.3 and sequel 4.44.0 (also see jeremyevans/sequel#1317)

When a LoadError is raised inside database.rb it is going to get "swallowed" which leads to errors way later in the application.

In my particular case a LoadError was raised before the database connection could be established. Since the loader just carried on, it finally crashed when loading a model (which do some introspection on their database)

While this behaviour is certainly useful for dealing with a common applications /lib directories, it hides very useful errors in important setup/init files.
Is there any way to exclude the database.rb from this (otherwise useful) loading behaviour?

@jorgefuertes

This comment has been minimized.

Show comment
Hide comment
@jorgefuertes

jorgefuertes Mar 10, 2017

Contributor

I'm using this approach at database.rb:

loop do
  begin
    MongoMapper.connection = Mongo::Connection.new('localhost', nil, logger: logger, pool_size: 16, pool_timeout: 30)
    logger.info 'MongoDB connection OK'
    break
  rescue StandardError => e
    logger.fatal 'MongoDB connection error! Please ensure Mongo is running!'
    logger.error e.message
    sleep 5
    logger.fatal 'Retrying...'
  end
end

case Padrino.env
when :development then MongoMapper.database = 'dbname_development'
when :production  then MongoMapper.database = 'dbname_production'
when :test        then MongoMapper.database = 'dbname_test'
end
Contributor

jorgefuertes commented Mar 10, 2017

I'm using this approach at database.rb:

loop do
  begin
    MongoMapper.connection = Mongo::Connection.new('localhost', nil, logger: logger, pool_size: 16, pool_timeout: 30)
    logger.info 'MongoDB connection OK'
    break
  rescue StandardError => e
    logger.fatal 'MongoDB connection error! Please ensure Mongo is running!'
    logger.error e.message
    sleep 5
    logger.fatal 'Retrying...'
  end
end

case Padrino.env
when :development then MongoMapper.database = 'dbname_development'
when :production  then MongoMapper.database = 'dbname_production'
when :test        then MongoMapper.database = 'dbname_test'
end
@fnordfish

This comment has been minimized.

Show comment
Hide comment
@fnordfish

fnordfish Mar 10, 2017

Contributor

Right. I could wrap my entire database.rb into a

begin
  # ...
rescue LoadError => e
  raise ScriptError.new(e) # or any other than which is not NameError or LoadError
end

But somehow, adding that stuff to avoid a framework specific behaviour feels weird (not very POLA)

Contributor

fnordfish commented Mar 10, 2017

Right. I could wrap my entire database.rb into a

begin
  # ...
rescue LoadError => e
  raise ScriptError.new(e) # or any other than which is not NameError or LoadError
end

But somehow, adding that stuff to avoid a framework specific behaviour feels weird (not very POLA)

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Mar 10, 2017

Member

Can not reproduce. Please publish a minimal failing (or in your case, not failing properly) project to Github.

Member

ujifgc commented Mar 10, 2017

Can not reproduce. Please publish a minimal failing (or in your case, not failing properly) project to Github.

@ujifgc ujifgc added the revalidate label Mar 10, 2017

@fnordfish

This comment has been minimized.

Show comment
Hide comment

@ujifgc ujifgc added bug reloader and removed revalidate labels Mar 10, 2017

@ujifgc ujifgc closed this in 843e474 Mar 13, 2017

@ujifgc ujifgc referenced this issue Mar 15, 2017

Closed

Release 0.14.0 #1971

adam12 added a commit to adam12/padrino-framework that referenced this issue Jun 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment