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 Organization.overquota exception logging #15873

Merged
merged 2 commits into from
Sep 28, 2020

Conversation

thedae
Copy link
Contributor

@thedae thedae commented Sep 28, 2020

Rollbar error: https://rollbar.com/carto/CartoDB/items/43042/ . Thanks @rafatower for reporting the exception.

Taking a look at this piece of code:

rescue Carto::Organization::OrganizationWithoutOwner => error
  log_warning(message: 'Skipping inconsistent organization', organization: self, exception: error)
  false
end

The problem is that the Carto::Organization.overquota static method was logging self as organization field in the log entry, but since this method is static, the field was the class itself, not the instance of Carto::Organization. The log looked like this:

{
  "organization": "Carto::Organization",
  "exception": {
    "class": "Carto::OrganizationCommons::OrganizationWithoutOwner",
    "message": "Organization carto-org has no owner",
    "backtrace_hint": [
      "/cartodb/app/models/carto/organization.rb:168:in `require_organization_owner_presence!'",
      "/cartodb/app/models/carto/organization.rb:63:in `get_geocoding_calls'",
      "/cartodb/app/models/carto/organization.rb:43:in `block in overquota'",
      "/var/lib/gems/2.5.0/gems/activerecord-4.2.11.3/lib/active_record/relation/batches.rb:51:in `block (2 levels) in find_each'",
      "/var/lib/gems/2.5.0/gems/activerecord-4.2.11.3/lib/active_record/relation/batches.rb:51:in `each'"
    ]
  },
  "timestamp": "2020-09-28 07:43:19 +0000",
  "levelname": "warning",
  "cdb-user": null,
  "event_message": "Skipping inconsistent organization"
}

I've replaced organization: self with organization: o and removed the message interpolation:

{
  "organization": "carto-org",
  "exception": {
    "class": "Carto::OrganizationCommons::OrganizationWithoutOwner",
    "message": "Organization has no owner",
    "backtrace_hint": [
      "/cartodb/app/models/carto/organization.rb:168:in `require_organization_owner_presence!'",
      "/cartodb/app/models/carto/organization.rb:63:in `get_geocoding_calls'",
      "/cartodb/app/models/carto/organization.rb:43:in `block in overquota'",
      "/var/lib/gems/2.5.0/gems/activerecord-4.2.11.3/lib/active_record/relation/batches.rb:51:in `block (2 levels) in find_each'",
      "/var/lib/gems/2.5.0/gems/activerecord-4.2.11.3/lib/active_record/relation/batches.rb:51:in `each'"
    ]
  },
  "timestamp": "2020-09-28 07:45:07 +0000",
  "levelname": "warning",
  "cdb-user": null,
  "event_message": "Skipping inconsistent organization"
}

@thedae thedae marked this pull request as ready for review September 28, 2020 08:07
Copy link
Contributor

@rafatower rafatower left a comment

Choose a reason for hiding this comment

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

👍

@rafatower rafatower merged commit 47e6c09 into master Sep 28, 2020
@rafatower rafatower deleted the fix/organizationwithoutowner-exception-too-deep branch September 28, 2020 09:11
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.

2 participants