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.

Sign up for GitHub

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

fix(insightsMiddleware): throw an error when credentials can't be extracted #4901

Merged
merged 3 commits into from
Sep 10, 2021
Merged

fix(insightsMiddleware): throw an error when credentials can't be extracted #4901

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
872c60b
fix(insightsMiddleware): throw an error when credentials can't be ext…
Haroenv Sep 8, 2021
09d9c8f
only dev mode, avoid bundle size bloat
Haroenv Sep 9, 2021
f812b80
can't be bothered with the ts
Haroenv Sep 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/lib/utils/getAppIdAndApiKey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// typed as any, since it accepts the _real_ js clients, not the interface we otherwise expect
export function getAppIdAndApiKey(searchClient: any) {
export function getAppIdAndApiKey(
searchClient: any
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
): [string | undefined, string | undefined] {
if (searchClient.transporter) {
// searchClient v4
const { headers, queryParameters } = searchClient.transporter;
Expand Down
36 changes: 26 additions & 10 deletions src/middlewares/__tests__/createInsightsMiddleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,22 @@ import { createSearchClient } from '../../../test/mock/createSearchClient';
import { warning } from '../../lib/utils';

describe('insights', () => {
const createTestEnvironment = () => {
const searchClientWithCredentials = createSearchClient({
// @ts-expect-error only available in search client v4
transporter: {
headers: {
'x-algolia-application-id': 'myAppId',
'x-algolia-api-key': 'myApiKey',
},
},
});
const createTestEnvironment = ({
searchClient = searchClientWithCredentials,
} = {}) => {
const { analytics, insightsClient } = createInsights();
const indexName = 'my-index';
const instantSearchInstance = instantsearch({
searchClient: createSearchClient({
// @ts-expect-error only available in search client v4
transporter: {
headers: {
'x-algolia-application-id': 'myAppId',
'x-algolia-api-key': 'myApiKey',
},
},
}),
searchClient,
indexName,
});
instantSearchInstance.start();
Expand Down Expand Up @@ -114,6 +117,19 @@ describe('insights', () => {
});
});

it('throws when search client does not have credentials', () => {
const { insightsClient, instantSearchInstance } = createTestEnvironment({
searchClient: createSearchClient(),
});
expect(() =>
createInsightsMiddleware({
insightsClient,
})({ instantSearchInstance })
).toThrowErrorMatchingInlineSnapshot(
`"[insights middleware]: could not extract Algolia credentials from searchClient"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable, but I'm curious in which case searchClient didn't include the credentials? In that case, InstantSearch.js itself won't be functioning, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you do the following:

const client = algoliasearch()
const searchClient = {
  search: client.search // or intercept
}

In that case only the "required" methods are present, as well as with a "backend search" without an algolia client the insights-thrown error is confusing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, that makes sense.

);
});

it('does not throw without userToken in UMD with the library loaded after the event', () => {
const {
insightsClient,
Expand Down
7 changes: 7 additions & 0 deletions src/middlewares/createInsightsMiddleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ export const createInsightsMiddleware: CreateInsightsMiddleware = (props) => {

return ({ instantSearchInstance }) => {
const [appId, apiKey] = getAppIdAndApiKey(instantSearchInstance.client);

if (!appId || !apiKey) {
throw new Error(
'[insights middleware]: could not extract Algolia credentials from searchClient'
);
}

let queuedUserToken: string | undefined = undefined;
let userTokenBeforeInit: string | undefined = undefined;

Expand Down
2 changes: 2 additions & 0 deletions src/widgets/hits/__tests__/hits-integration-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const createSearchClient = ({
),
})
),
applicationID: 'latency',
apiKey: '123',
};
};

Expand Down
2 changes: 2 additions & 0 deletions src/widgets/infinite-hits/__tests__/infinite-hits-integration-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ describe('infiniteHits', () => {
),
}) as any
),
// credentials are stored like this in client v3, but not part of the SearchClient type
...({ applicationID: 'latency', apiKey: '123' } as any),
});
const search = instantsearch({
indexName: 'instant_search',
Expand Down