-
Notifications
You must be signed in to change notification settings - Fork 133
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
feat: add sendBeacon support #412
Changes from all commits
80744cf
f708c94
698df2a
9e7d1f6
0222136
7902a98
c2a6222
006352c
bfad086
8646f90
ecce8b6
e989cb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -212,6 +212,31 @@ AmplitudeClient.prototype.init = function init(apiKey, opt_userId, opt_config, o | |
if (type(opt_callback) === 'function') { | ||
opt_callback(this); | ||
} | ||
|
||
const onExitPage = this.options.onExitPage; | ||
if (type(onExitPage) === 'function') { | ||
if (!this.pageHandlersAdded) { | ||
this.pageHandlersAdded = true; | ||
|
||
const handleVisibilityChange = () => { | ||
const prevTransport = this.options.transport; | ||
this.setTransport(Constants.TRANSPORT_BEACON); | ||
onExitPage(); | ||
this.setTransport(prevTransport); | ||
}; | ||
|
||
// Monitoring just page exits because that is the most requested feature for now | ||
// "If you're specifically trying to detect page unload events, the pagehide event is the best option." | ||
// https://developer.mozilla.org/en-US/docs/Web/API/Window/pagehide_event | ||
window.addEventListener( | ||
'pagehide', | ||
() => { | ||
handleVisibilityChange(); | ||
}, | ||
false, | ||
); | ||
} | ||
} | ||
} catch (err) { | ||
utils.log.error(err); | ||
if (type(opt_config.onError) === 'function') { | ||
|
@@ -334,7 +359,9 @@ var _parseConfig = function _parseConfig(options, config) { | |
|
||
var inputValue = config[key]; | ||
var expectedType = type(options[key]); | ||
if (!utils.validateInput(inputValue, key + ' option', expectedType)) { | ||
if (key === 'transport' && !utils.validateTransport(inputValue)) { | ||
return; | ||
} else if (!utils.validateInput(inputValue, key + ' option', expectedType)) { | ||
return; | ||
} | ||
if (expectedType === 'boolean') { | ||
|
@@ -511,6 +538,13 @@ AmplitudeClient.prototype._sendEventsIfReady = function _sendEventsIfReady() { | |
return true; | ||
} | ||
|
||
// if beacon transport is activated, send events immediately | ||
// because there is no way to retry them later | ||
if (this.options.transport === Constants.TRANSPORT_BEACON) { | ||
this.sendEvents(); | ||
return true; | ||
} | ||
|
||
// otherwise schedule an upload after 30s | ||
if (!this._updateScheduled) { | ||
// make sure we only schedule 1 upload | ||
|
@@ -960,6 +994,25 @@ AmplitudeClient.prototype.setDeviceId = function setDeviceId(deviceId) { | |
} | ||
}; | ||
|
||
/** | ||
* Sets the network transport type for events. Typically used to set to 'beacon' | ||
* on an end-of-lifecycle event handler such as `onpagehide` or `onvisibilitychange` | ||
* @public | ||
* @param {string} transport - transport mechanism to use for events. Must be one of `http` or `beacon`. | ||
* @example amplitudeClient.setDeviceId('45f0954f-eb79-4463-ac8a-233a6f45a8f0'); | ||
*/ | ||
AmplitudeClient.prototype.setTransport = function setTransport(transport) { | ||
if (this._shouldDeferCall()) { | ||
return this._q.push(['setTransport'].concat(Array.prototype.slice.call(arguments, 0))); | ||
} | ||
|
||
if (!utils.validateTransport(transport)) { | ||
return; | ||
} | ||
|
||
this.options.transport = transport; | ||
}; | ||
|
||
/** | ||
* Sets user properties for the current user. | ||
* @public | ||
|
@@ -1609,11 +1662,13 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() { | |
|
||
// We only make one request at a time. sendEvents will be invoked again once | ||
// the last request completes. | ||
if (this._sending) { | ||
return; | ||
// beacon data is sent synchronously, so don't pause for it | ||
if (this.options.transport !== Constants.TRANSPORT_BEACON) { | ||
if (this._sending) { | ||
return; | ||
} | ||
this._sending = true; | ||
} | ||
|
||
this._sending = true; | ||
var protocol = this.options.forceHttps ? 'https' : 'https:' === window.location.protocol ? 'https' : 'http'; | ||
var url = protocol + '://' + this.options.apiEndpoint; | ||
|
||
|
@@ -1633,6 +1688,19 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() { | |
checksum: md5(Constants.API_VERSION + this.options.apiKey + events + uploadTime), | ||
}; | ||
|
||
if (this.options.transport === Constants.TRANSPORT_BEACON) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be too much, but would be good to replace a lot of these with a small descriptive helper function like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried adding helper functions, but since there are only two transports now, they didn't add much descriptivity compared to checking the value itself. If there end up being more than two, then definitely there should be helper functions to categorize them like this |
||
const success = navigator.sendBeacon(url, new URLSearchParams(data)); | ||
|
||
if (success) { | ||
this.removeEvents(maxEventId, maxIdentifyId, 200, 'success'); | ||
if (this.options.saveEvents) { | ||
this.saveEvents(); | ||
} | ||
} else { | ||
this._logErrorsOnEvents(maxEventId, maxIdentifyId, 0, ''); | ||
} | ||
return; | ||
} | ||
var scope = this; | ||
new Request(url, data, this.options.headers).send(function (status, response) { | ||
scope._sending = false; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,6 +108,23 @@ const validateDeviceId = function validateDeviceId(deviceId) { | |
return true; | ||
}; | ||
|
||
const validateTransport = function validateTransport(transport) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this checked on initialization? In case it comes in bad There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's not but it should be, good call |
||
if (!validateInput(transport, 'transport', 'string')) { | ||
return false; | ||
} | ||
|
||
if (transport !== constants.TRANSPORT_HTTP && transport !== constants.TRANSPORT_BEACON) { | ||
log.error(`transport value must be one of '${constants.TRANSPORT_BEACON}' or '${constants.TRANSPORT_HTTP}'`); | ||
return false; | ||
} | ||
|
||
if (transport !== constants.TRANSPORT_HTTP && !navigator.sendBeacon) { | ||
log.error(`browser does not support sendBeacon, so transport must be HTTP`); | ||
return false; | ||
} | ||
return true; | ||
}; | ||
|
||
// do some basic sanitization and type checking, also catch property dicts with more than 1000 key/value pairs | ||
var validateProperties = function validateProperties(properties) { | ||
var propsType = type(properties); | ||
|
@@ -269,4 +286,5 @@ export default { | |
validateInput, | ||
validateProperties, | ||
validateDeviceId, | ||
validateTransport, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thoughts on hooking the current object with
onExitPage(this)
allowing users to access the client earlier?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in, so they can write the callback on a line before initializing the client with it? The way I was testing it, as long as the amplitude object is in scope within the file, it doesn't need to be initialized until onExitPage is called. So the following is fine:
But if there's another benefit you're thinking of by making the client an argument, it would be an easy change to make
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might make it easier for people using the snippet to use the full client obj (incl. "private" methods and the full list of options post-init to construct event properties) but you are right that everything publicly supported is available.
I don't feel too strongly about this though so we can not do this and see what people actually want as arguments to this option!