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

Fixed number of arguments in Puma::Events#unknown_error #58

Closed
wants to merge 1 commit into from

Conversation

paneq
Copy link

@paneq paneq commented Apr 7, 2012

The method is actually called with 4 arguments : https://github.com/puma/puma/blob/master/lib/puma/server.rb#L289
Even the documentation of it mentions the env argument.

@seamusabshere
Copy link
Contributor

this should fix #54

@@ -33,7 +33,7 @@ def parse_error(server, env, error)
# +server+ is the Server object, +env+ the request, +error+ an exception
# object, and +kind+ some additional info.
#
def unknown_error(server, error, kind="Unknown")
def unknown_error(server, env, error, kind="Unknown")
Copy link
Contributor

Choose a reason for hiding this comment

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

hey @paneq, you also need to fix line 184 of server.rb:

lib/puma/server.rb:
  182                client.close rescue nil
  183              rescue Object => e
  184:               @events.unknown_error self, e, "Listen loop"
  185              end
  186            end

You could change it to

@events.unknown_error self, nil, e, "Listen loop"

@seamusabshere
Copy link
Contributor

hey @evanphx it's also possible that you don't want #unknown_error to accept env... either way, here's where the problems are:

lib/puma/server.rb:
  182                client.close rescue nil
  183              rescue Object => e
  184:               @events.unknown_error self, e, "Listen loop"  # <- 3 args
  185              end
  186            end
  ...
  287        # Server error
  288        rescue StandardError => e
  289:         @events.unknown_error self, env, e, "Read"          # <- 4 args
  290  
  291        ensure
  ...
  295            # Already closed
  296          rescue StandardError => e
  297:           @events.unknown_error self, env, e, "Client"      # <- 4 args
  298          end
  299        end

@evanphx
Copy link
Member

evanphx commented Apr 9, 2012

Yep, I'll have this fixed today and released. Thanks!

evanphx added a commit that referenced this pull request Apr 10, 2012
@evanphx
Copy link
Member

evanphx commented Apr 10, 2012

Fixed by 17219d9

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