getSource does not always use browser cache when it should

[eriwen]

See stacktracejs/stacktrace.js#132

[orf]

So I first discovered this while making an emberjs project, this is an EmberJS route that can trigger the issue on both Firefox and Chrome:

import Ember from 'ember';

export default Ember.Route.extend({
    init(){
        try{
            throw new Error();
        } catch (e){
            StackTrace.fromError(e).then(console.log);
        }
    }
});

Just make an empty Ember project (npm install -g ember-cli && mkdir stacktrace-test && cd stacktrace-test/ && ember init), make an empty route (ember g route error) and paste the code above in the app/routes/error.js file. Oh, and paste the CDN link to stacktrace.js inside app/index.html.

Then serve with ember serve, visit http://localhost:4200/error and you should see your browser freeze for about 30 seconds as it loads the .map and .js files a bunch of times.

I think the fix should be simple, in line 166 you should perhaps have a separate cache for 'in flight requests' or such, and then check if this URL is in the in-flight request list before initiating another request. Not sure how this is going to work with stacktrace-js though.

You mentioned in the previous ticket that you rely on the browsers cache to do this, but did this ever work? As far as I can tell each call to getMappedLocation initiates another Ajax request, which the browser will initiate, the server could reply with a cached response but that won't help if there are 20 requests pending for the same resource.

[oliversalzburg]

Which headers are sent with the response for these files? If the wrong cache control headers are sent, that would explain the behavior. Re-implementing caching when browsers are capable of handling it, doesn't sound like a good idea.

[orf]

Re-implementing caching when browsers are capable of handling it, doesn't sound like a good idea.

But then why have a sourceCache at all? Irregardless of caching the current issue is this: A stacktrace with a depth of 30 makes 30*2 network requests (1 for the js, 1 for the map per line). This is madness, even with proper server caching the latency is excessive (even more so for anyone on a low-bandwidth connection). If you make this.ajax() return a promise then you can just cache the promise in the sourceCache until it is resolved, so any future invocations of _get() just wait for that single in-flight AJAX request to resolve and use the response, rather than each issuing their own and hoping it all works out.

All this could be done with a small fix to _get() (untested, just to illustrate):

if (this.sourceCache[location]) {
    if (this.sourceCache[location] instanceof Promise){
        this.sourceCache[location].then(resolve).error(reject);
    } else {
        resolve(this.sourceCache[location]);
    }
} else if (opts.offline && !isDataUrl) {
    ...
} else {
    if (isDataUrl) {
        ...
    } else {
        this.sourceCache[location] = this.ajax(location, function (source) {
            this.sourceCache[location] = source;
            resolve(source);
        }.bind(this), reject);
    }
}

After the future is resolved the sourceCache is updated with the content, so any future invocations just use that response.

This way as long as the sourceCache is passed from stacktrace.js multiple calls to stacktrace-gps will share the cache, thus share in-flight requests, and so only need to make perhaps 2 or 3 (depending on how many different URL's you have to fetch) requests instead of 60.

[oliversalzburg]

But then why have a sourceCache at all?

That's a valid point. If browser caching is utilized correctly, no request will ever leave the browser, as the resource is served from the browser cache.

But, maybe you're right, even if the correct headers aren't sent to signal that the resource shouldn't be cached, there is really no advantage for stacktrace-gps to retrieve the map again. If the map changed during requests, it wouldn't match the original source anyway.

[orf]

Exactly, you also have to consider this: by default the .map files won't be cached. So you have a stacktrace with 30 lines, the first request will be uncached but will only return after the other 29 are in-flight, meaning the first time you try and format a stack trace the browsers cache is useless anyway (as the browser can't cancel/modify the in-flight requests).

The fix is really simple though, as stated above: just take advantage of promises. I could take a crack at an actual fix if you want?