Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Added a configuration option for setting path to notifier.js #229

Closed
wants to merge 2 commits into from

4 participants

Vince Puzzella Ben Arent Duncan Beevers Marko Šiftar
Vince Puzzella

No description provided.

Ben Arent
Owner

@duncanbeevers What are your thoughts on this / moving forwards as we update the javascripit app?

Duncan Beevers

The way the JS notifier is written, it's well decoupled from the whatever app its used in. Specifically, the notifier reports directly to the Airbrake API servers via a JSONP GET, avoiding cross-origin request restrictions.

If people want to bundle the javascript into their app JS payload that should be easy, and the mechanism for doing so should be provided out of the box. This approach, while flexible, seems a little dangerous. People have three options for providing the JS notifier:

  1. Link to a remote script
  2. Bundle a local script
  3. Link to a local script

In all of those cases, we don't really want people futzing with client-side code. If they want to do that, they should pull the client side code out of the gem and put it into app/assets/javascripts with their local modifications.

Explicitly specifying the load path but not providing control of the loading mechanism (async download via document.write) sort of conflates the loading with the functionality of the script itself.

In short, I like the knobs, but I think they're on the wrong box.

Ben Arent
Owner

@duncanbeevers cool, thanks for the clarification.

Ali Faiz alif referenced this pull request
Merged

Remove JS notifier #297

Marko Šiftar
Owner

Closing this since we removed Javascript notifier from the gem.
@vpuzzella please refer to https://github.com/airbrake/airbrake-js for further changes.
Thank you.

Marko Šiftar shifi closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 18, 2013
  1. Vince Puzzella
Commits on Jul 19, 2013
  1. Vince Puzzella

    Nil-proof string concatenation in Airbrake::Rails::JavascriptNotifier…

    vpuzzella authored
    …#airbrake_javascript_notifier
This page is out of date. Refresh to see the latest.
6 lib/airbrake/configuration.rb
View
@@ -3,7 +3,7 @@ module Airbrake
class Configuration
OPTIONS = [:api_key, :js_api_key, :backtrace_filters, :development_environments,
- :development_lookup, :environment_name, :host,
+ :development_lookup, :environment_name, :host, :path,
:http_open_timeout, :http_read_timeout, :ignore, :ignore_by_filters,
:ignore_user_agent, :notifier_name, :notifier_url, :notifier_version,
:params_filters, :project_root, :port, :protocol, :proxy_host,
@@ -23,6 +23,9 @@ class Configuration
# The host to connect to (defaults to airbrake.io).
attr_accessor :host
+ # The path to to notifier.js to (defaults to /javascripts).
+ attr_accessor :path
+
# The port on which your Airbrake server runs (defaults to 443 for secure
# connections, 80 for insecure connections).
attr_accessor :port
@@ -161,6 +164,7 @@ def initialize
@secure = false
@use_system_ssl_cert_chain= false
@host = 'api.airbrake.io'
+ @path = '/javascripts'
@http_open_timeout = 2
@http_read_timeout = 5
@params_filters = DEFAULT_PARAMS_FILTERS.dup
3  lib/airbrake/rails/javascript_notifier.rb
View
@@ -10,7 +10,7 @@ def self.included(base) #:nodoc:
private
def airbrake_javascript_notifier
- airbrake_javascript_loader + airbrake_javascript_configuration
+ airbrake_javascript_loader.to_s + airbrake_javascript_configuration.to_s
end
def airbrake_javascript_loader
@@ -41,6 +41,7 @@ def airbrake_javascript_configuration
def _airbrake_render_part(path, locals={})
locals[:host] = _airbrake_host
+ locals[:path] = Airbrake.configuration.path
options = {
:file => path,
2  lib/templates/javascript_notifier_loader
View
@@ -1,6 +1,6 @@
<%= javascript_tag %Q{
(function(){
var notifierJsScheme = (("https:" == document.location.protocol) ? "https://" : "http://");
- document.write(unescape("%3Cscript src='" + notifierJsScheme + "#{host}/javascripts/notifier.js' type='text/javascript'%3E%3C/script%3E"));
+ document.write(unescape("%3Cscript src='" + notifierJsScheme + "#{host}#{path}/notifier.js' type='text/javascript'%3E%3C/script%3E"));
})();
}%>
4 test/configuration_test.rb
View
@@ -17,6 +17,7 @@ class ConfigurationTest < Test::Unit::TestCase
assert_config_default :notifier_url, 'https://github.com/airbrake/airbrake'
assert_config_default :secure, false
assert_config_default :host, 'api.airbrake.io'
+ assert_config_default :path, '/javascripts'
assert_config_default :http_open_timeout, 2
assert_config_default :http_read_timeout, 5
assert_config_default :ignore_by_filters, []
@@ -76,6 +77,7 @@ class ConfigurationTest < Test::Unit::TestCase
assert_config_overridable :proxy_pass
assert_config_overridable :secure
assert_config_overridable :host
+ assert_config_overridable :path
assert_config_overridable :port
assert_config_overridable :http_open_timeout
assert_config_overridable :http_read_timeout
@@ -98,7 +100,7 @@ class ConfigurationTest < Test::Unit::TestCase
config = Airbrake::Configuration.new
hash = config.to_hash
[:api_key, :backtrace_filters, :development_environments,
- :environment_name, :host, :http_open_timeout,
+ :environment_name, :host, :path, :http_open_timeout,
:http_read_timeout, :ignore, :ignore_by_filters, :ignore_user_agent,
:notifier_name, :notifier_url, :notifier_version, :params_filters,
:project_root, :port, :protocol, :proxy_host, :proxy_pass, :proxy_port,
Something went wrong with that request. Please try again.