matomo-org / matomo-nodejs-tracker

A Node.js wrapper for the Matomo (Piwik) tracking HTTP API
MIT License
117 stars 41 forks source link

Should not fail on trackerUrl.endsWith("matomo.php") (or piwik) #44

Closed MoOx closed 6 years ago

MoOx commented 6 years ago

This should be optional as we can change this (eg url rewriting to avoid ad-blockers).

I think instead of assertion, this lib should just warn.

Findus23 commented 6 years ago

Hi @MoOx,

I understand that forcing a specific file ending is a bit extreme, but I am not sure what a solution could be. Removing it will cause a lot of issues, who will try the root URL while replacing it with a warning, will always show a warning in cases like yours even though nothing is wrong.

The iOS SDK also seems to force a suffix: https://github.com/matomo-org/matomo-sdk-ios/blob/904987821936762137eb984c13350351bc1a07eb/PiwikTracker/PiwikTracker.swift#L79

Do you have an idea for an proper solution? Maybe an extra parameter?

MoOx commented 6 years ago

An extra parameter to avoid the check would be awesome. Like "I know what I am doing". What about

// default values
avoidValidation: {
  siteId: false,
  trackerUrl: false,
  url: false,
}

(I am only interested about trackerUrl, but I guess it's not a big deal to cover all assertions?)

MoOx commented 6 years ago

Or the opposite:

// default values
validateArgs: {
  siteId: true,
  trackerUrl: true,
  url: true,
}
Findus23 commented 6 years ago

Hi, I have created #45 with an solution. I don't think it is useful to allow siteId that's neither a string nor a integer or a trackerUrl that isn't an string.

Findus23 commented 6 years ago

Released 2.1.0 including the fix