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

Asynchronous Loading and Capturing Errors #169

Closed
karolisdzeja opened this issue Dec 18, 2013 · 36 comments
Closed

Asynchronous Loading and Capturing Errors #169

karolisdzeja opened this issue Dec 18, 2013 · 36 comments

Comments

@karolisdzeja
Copy link

I want to be able to load raven.js asynchronously, but still be able to capture errors while the script is loading. (Something like how Google Analytics handles events by storing them in a variable until the library loads).

Here's what I have so far: https://gist.github.com/karolisdzeja/8010574

But doing it that way loses some of the information and detail that Raven usually provides. Is there a way to store the full error information similarly to this?

@PatrickJS
Copy link

+1

3 similar comments
@nathmisaki
Copy link

+1

@Sija
Copy link
Contributor

Sija commented Dec 9, 2015

+1

@sodevious
Copy link

+1

@benvinegar
Copy link
Contributor

Everyone who's commented on this – what's wrong with the solution by @karolisdzeja?

Ultimately, I'm not sure how we can add a feature to the Raven.js source that is supposed to work when Raven.js source isn't on the page. I think this will always ultimately be a custom solution; at best we could add a "how to" to our docs.

@jwheare
Copy link

jwheare commented Jun 24, 2016

@benvinegar the solution looks fine, but it would be better if this was officially supported and documented. I'm happier just having to trust the Sentry team over evaluating a random gist.

@jwheare
Copy link

jwheare commented Jun 24, 2016

Actually, a much better solution would be something like Twitter's JS SDK code: https://dev.twitter.com/web/javascript/loading

Setup a function queue on page load that then gets consumed when the external js has been loaded, replacing the proxy object. And make sure all API calls go through something like a .ready() call to the proxy.

This ensures that any call can be queued up before the js has loaded rather than just captureMessage, without having to proxy every function individually.

I'd love to be able to just load raven.js asynchronously/deferred and not have to worry.

@jwheare
Copy link

jwheare commented Jun 24, 2016

Other problems with the gist: it clobbers window.onerror and introduces a few uncontained global variables.

@jwheare
Copy link

jwheare commented Jun 24, 2016

It also doesn't make use of the full-featured traceKitWindowOnError function that raven.js uses when it loads.

@oroce
Copy link

oroce commented Nov 11, 2016

I slightly redid the gist above: https://gist.github.com/oroce/ec3786ba7eff59963842220c3ffc56b4

There's no leaking variable. I kept the window.onerror handler, but feel free to use window.addEventListener('error', fn).

The biggest help at point be having traceKitWindowOnError as an exported function from Raven. Since that's the function which gets called when an error happens: https://github.com/getsentry/raven-js/blob/master/dist/raven.js#L2074

I know we wouldn't have that very proper stacktrace but we would have something better than what we have now.

@benvinegar
Copy link
Contributor

There's other downsides with doing this:

  • By relying on window.onerror to catch errors before Raven is loaded, stacktraces are not available for every browser
    • besides not having a stack trace, this will affect grouping negatively
    • this is why install() does try/catch instrumentation
  • Synthetic traces will not work (they will all appear as originating from this code)
  • No breadcrumb collection

So, you're trading potentially better performance for lower quality error reports. If async execution is important, I'd sooner recommend you bundle Raven with your own code so that it is served together.

@oroce
Copy link

oroce commented Nov 23, 2016

@benvinegar you are completely right. In applications which aren't public (aka google won't reach the pages) the classic (blocking) raven way is completely fine but as soon as you have a public facing site where the google page insight points do matter, we need to optimize how we load third party code (this is the price we are willing to pay in favour of ux, speed and better search result position).

Moreover bundling raven into our bundle is a solution, but as soon as you get into optimizing your frontend code with above the fold optimizations, like splitting your bundle into multiple ones using tools like factor-bundle or you include multiple bundles to gain more speed, the above solution can be a better one imo, but I'm open for suggestions.

@vinhlh
Copy link

vinhlh commented Dec 5, 2016

They all have trade-offs, so it would be great if we can document all available strategies, so depend on per specific web application, we will apply different strategies.
With async strategy, we should load the script after onload event, instead of loading async only, to prevent blocking onload event, rendering.

/**
 * Setup Js error lazy tracking
 * - Pros: doesn't block rendering, onload event
 * - Cons: lower quality error reports for lazy errors
 *
 * @author vinhlh
 *
 * @param  {object} window
 * @param  {object} labJs
 * @param  {string} ravenCdn
 * @param  {string} sentryDsn
 */
(function(window, labJs, ravenCdn, sentryDsn) {
  var errors = [];
  var oldOnError = window.onerror;

  window.onerror = function() {
    errors.push(arguments);
    oldOnError && oldOnError.apply(this, arguments);
  };
  window.addEventListener('load', function() {
    labJs
      .script(ravenCdn)
      .wait(function() {
        window.onerror = oldOnError;
        Raven.config(sentryDsn).install();
        errors.forEach(function(args) {
          window.onerror.apply(this, args);
        });
      });
  });
})(window, $LAB, 'raven-js-3.8.1/dist/raven.js', 'https://xxx@domain.com/9');

@benvinegar
Copy link
Contributor

benvinegar commented Dec 9, 2016

I think we'll probably just document an async snippet, like the ones provided above, but mention that it comes with tradeoffs.

Just one other comment. Those tradeoffs might seem acceptable, but I deal with a lot of support tickets from users about low-fidelity errors they are experiencing that are (incorrectly) believed to derive from Raven.js. My fear is that if I encourage people to use the async approach, I'll have more and more people asking me "why is there no stack trace" and other complaints when it's because this approach is lower fidelity. I'm willing to accept that, but it's a tough pill to swallow. 😓

@oroce
Copy link

oroce commented Dec 9, 2016

@benvinegar I totally get where you are coming from, I know what I'm doing so I don't expect stacktrace when there shouldn't be. For newcomers, less experienced ones or just users with different usecase it can be leading for sure.

@benvinegar
Copy link
Contributor

@oroce – yeah, this is 100% not a concern with people in this thread, but people who might pursue this strategy without understanding the caveats properly (e.g. just copy/pasting).

I'll keep this issue open, with a plan to add the snippet to the Install docs – and I'll put a bunch of warnings all over the place.

Thanks again for your participation here / convincing me to do this.

@kl0tl
Copy link

kl0tl commented Dec 15, 2016

Here’s the snippet we use to queue calls to Raven methods transparently: https://gist.github.com/Kl0tl/ed0a9e74462a2294f4c8842f5389d8ea.

The mock could certainly be improved but we don’t need to replicate more functionalities. Object.defineProperty allows us to hook right after Raven attaches itself to the window but maybe the script load event is enough. Would be nice to have an official way to enable this.

@zanona
Copy link

zanona commented Mar 2, 2017

Hey guys, just wondering if there is anything wrong with the way Raygun does that asynchronously?
I am not sure, but it seems to handle edge cases well? I might be wrong though :)

@mqklin
Copy link

mqklin commented Mar 20, 2017

@kl0tl very nice, thank you

@pladaria
Copy link

pladaria commented Aug 28, 2017

This is very simple using a dynamic import. Still in stage3 but supported by webpack.

We simply use the promise as a queue. Once fullfiled all callbacks are executed in order.

const RavenPromise = import('raven-js'); // async load raven bundle

// initial setup
RavenPromise.then(Raven => {
    Raven.config('url-to-sentry', options).install();
}):

// exported log function
export const logMessage = (level, logger, text) => {
    RavenPromise.then(Raven => {
        Raven.captureMessage(text, {level, logger});
    });
};

@kireerik
Copy link

kireerik commented Nov 27, 2017

Similarly to @zanona I would also prefer to have a simple tracking code like Raygun or Google Analytics has. Here is an example for analytics.js:

<script async src="https://www.google-analytics.com/analytics.js"></script>
<script>
	window.ga = window.ga || function () {
		(ga.q = ga.q || []).push(arguments)
	}
	ga.l = +new Date

	ga('create', 'UA-XXXXX-Y', 'auto')
	ga('send', 'pageview')
</script>

@benvinegar Does something like this possible with Raven.js? Maybe in the future?

@kamilogorek
Copy link
Contributor

@kireerik it definitely will be implemented (most likely as a documentation how-to), but I cannot give you an accurate date estimate now.

@kireerik
Copy link

kireerik commented Dec 6, 2017

@kamilogorek Sounds great (I don't like the workaround as a solution). No problem!

@maxmilton
Copy link

For anyone interested, I've put up a gist of yet another way to load ravenjs asynchronously:
https://gist.github.com/MaxMilton/e2338b02b7381fc7bef2ccd96f434201

It's based on the code by @oroce but with the key differences being I use a regular <script async src'='..."> tag in the document head for better performance (browsers can schedule fetching the resource earlier) + I don't bother wrapping it in an IIFE and other little tweaks.

@kireerik
Copy link

kireerik commented Jan 14, 2018

@maxmilton Nice one! I have created my own flavor based on yours:

<script async src="https://cdn.ravenjs.com/3.22.1/raven.min.js" crossorigin="anonymous" id="raven"></script>
<script>
	(function (sentryDataSourceName) {
		var raven = document.getElementById('raven')
		, isNotLoaded = true
		, errors = []
		raven.onreadystatechange = raven.onload = function () {
			if (isNotLoaded) {
				Raven.config(sentryDataSourceName).install()
				isNotLoaded = !isNotLoaded
				errors.forEach(function (error) {
					Raven.captureException(error[4] || new Error(error[0]), {
						extra: {
							file: error[1]
							, line: error[2]
							, col: error[3]
						}
					})
				})
			}
		}
		window.onerror = function (message, source, lineNumber, colmnNumber, error) {
			if (isNotLoaded)
				errors.push([message, source, lineNumber, colmnNumber, error])
		}
	})('https://<key>@sentry.io/<project>')
</script>

I also have some question:

  • Is it needed to define the crossorigin attribute on the script tag?
  • Is it enough to pass jsut the error object instead of the other solution?

What do you think? What is the author's (@kamilogorek) opinion on this?

@maxmilton
Copy link

maxmilton commented Jan 14, 2018

@kireerik when you put crossorigin="anonymous" on scripts it allows you to fully capture errors (from that external script) with the window.onerror event. It also prevents the browser from sending credentials with the fetch request, which is typically what you want with 3rd party resources. MDN reference 1, MDN reference 2.

You can just pass the error and it will work most of the time. The caveat being old browsers (e.g. Firefox before version 31) don't pass the columnNo or Error Object properties to the window.onerror event. So if you want really good compatibility then you need to do that extra bit. MDN reference.

EDIT: Bonus tip: turns out when you put crossorigin without any value it's treated the same as crossorigin="anonymous".

@maxmilton
Copy link

maxmilton commented Jan 14, 2018

FYI, I've updated my previous gist to something that's much more production ready:

  • lots of comments to explain what's actually going on
  • big clean up + use descriptive variables names (always a nice bonus 😉 )
  • wrap in IIFE to not pollute the global namespace
  • fix incorrect params passed to error array item

See the gist if you want to understand everything OR if you prefer a quick copy+paste, here's the minified version:

<!-- Sentry JS error tracking -->
<script async src="https://cdn.ravenjs.com/3.22.0/raven.min.js" crossorigin id="raven"></script>
<script>
(function(b,e,c,d){b.onerror=function(a,b,d,f,g){c||e.push([a,b,d,f,g])};b.onunhandledrejection=function(a){c||e.push([a.reason.reason||a.reason.message,a.type,JSON.stringify(a.reason)])};d.onreadystatechange=d.onload=function(){c||(c=!0,
Raven.config("___PUBLIC_DSN___").install(),
b.onunhandledrejection=function(a){Raven.captureException(Error(a.reason.reason||a.reason.message),{extra:{type:a.type,reason:JSON.stringify(a.reason)}})},e.forEach(function(a){Raven.captureException(a[4]||Error(a[0]),{extra:{file:a[1],line:a[2],col:a[3]}})}))}})(window,[],!1,document.getElementById("raven"));
</script>

<link rel="preconnect" href="https://sentry.io">

Replace ___PUBLIC_DSN___ with your DSN and paste this somewhere in the head near your closing </head> tag. Or if you're a hipster who doesn't use <head> and <body> tags anymore then just paste it near the top after any critical/app resources (e.g. CSS). Ideally it should be before any other JavaScript so you can capture errors from scripts loaded after this code.

In my quick tests there hasn't been any issues so I don't see any reason not to use this over the default synchronous version.

If anyone has an idea for a better approach I'm keen to hear it.

Edit: Sorry for editing this comment so many times. It's at a stable level now. Was fun getting it to this state! 😃

@kamilogorek
Copy link
Contributor

https://docs.sentry.io/clients/javascript/install/#async-loading

@dalyr95
Copy link

dalyr95 commented Jul 2, 2018

Once the sentry library is loaded, error reporting quality is exactly the same as loading it sync? (I assume so, just checking)

@dalyr95
Copy link

dalyr95 commented Jul 2, 2018

Also in the docs you might want to add, you can't use Raven till the lib is loaded, maybe provide a callback function in the options so you can set user context etc?

@lili21
Copy link

lili21 commented Jul 5, 2018

agree with @dalyr95 . a callback function would be useful. maybe a custom raven load event.

@xt0rted
Copy link

xt0rted commented Jul 16, 2018

I have a similar request as @dalyr95. Right now the only way to call setUserContext() is to modify the loader snippet which isn't as clean as being able to pass in a callback on the main config object.

@kamilogorek
Copy link
Contributor

Hello, thanks for reporting the issue.

We are in the process of working on the next major release of our SDK.
Because of that, we had to put working on the current version (except major or security bugs) on hold.
We'll try to get back to all the reports as soon as possible, so please be patient.

Thanks for your understanding,
Cheers!

@xt0rted
Copy link

xt0rted commented Jul 17, 2018

My solution was to add 'undefined'!=k.setup&&k.setup(); immediately after the call to .install(), then I added a function called setup to SENTRY_SDK with my post init code.

@danielcompton
Copy link

With the async loader, I was able to set the user context and other info by passing it as a second argument to Raven.config:

<script>
    Raven.config("https://<mydsn>@sentry.io/<projectid>", 
      {"release":"0.3.1",
       "environment":"dev",
       "user": {"id":"7b031fa0-32ff-46fe-b94b-e6bc201c3c5f",
                "organisation-id":"b1a50c41-b85e-4c50-9cec-c55ff36cf6d1"}}).install();
</script>

I think everything already exists for this, it just could be better documented.

@lili21
Copy link

lili21 commented Jul 19, 2018

@danielcompton can only get user information through backend api ?

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

No branches or pull requests