fix: prevent error when calling upload() early

[mmustafa-tse]

Summary

• github issue with description here

Testing Plan

• [Y] Was this tested locally? If not, explain why.
• Tested with local overrides

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

• Closes https://mparticle-eng.atlassian.net/browse/SQDSDKS-6164

[alexs-mparticle]

Can you write a test that catches this?

[mmustafa-tse]

just added the test case, thnx!

[rmi22186]

there exists a case where _APIClient won't yet be available, when there is a /config call in a self hosted world, so let's add a ? to _APIClient also:

Suggested change

	self._APIClient.uploader?.prepareAndUpload(false, false);
	self._APIClient?.uploader?.prepareAndUpload(false, false);

[alexs-mparticle]

I think you should add an assertion to verify that an error actually doesn't throw.

IIRC the format should be something like:
expect(() => { window.mParticle.upload()}).not.to.throw(new Error('<ERROR MESSAGE>'))
or
expect(() => { window.mParticle.upload()}).not.to.throw('<ERROR MESSAGE>')

I think the gist is to make sure we're actually catching the right error message or to make sure throwing an error doesn't suspend other testing.

I would timebox this unless @rmi22186 feels strongly about it as well.

[rmi22186]

yea - i actually was thinking the same thing, assuming that if an error threw, perhaps the test would still pass since there is no expect. however, i did pull the code down yesterday to check and it throws an error and the test fails, so at least it's not a silent error.

here's a recommendation i found online as an example of how this would work, so try this, and if it doesn't work, I think we are good to go with what you currently have, and just add a comment above it that says something like

const { expect } = require('chai');
const sinon = require('sinon');

describe('MyFunction', () => {
    it('should not throw an error', () => {
        const myFunction = sinon.stub().returns(true); // Stub the function
        
        expect(() => myFunction()).to.not.throw();
    });
});

and if that doesn't work, then put a comment explaining why there is not expect

 // the following does not throw an error, but if it did, the test would fail because the error is not silent
or something like that. 

[mmustafa-tse]

I was able to get it working and not throw the error with a regex matching a part of the message while specifying the error type to be TypeError, tested on both before and after the fix and it worked as expected

[sonarcloud]

Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[rmi22186]

I pulled the code down to run the new test undoing the source code change, and it works great