FP S4123 no-invalid-await on JS file in SonarLint

[peterpoliwoda]

Reflecting on a forum issue spoken about here:
https://community.sonarsource.com/t/refactor-this-redundant-await-on-a-non-promise-is-not-correct/28800

My VSCode extension wrongly tells me that I don't need to await for a non-promise. Problem is, my function is an async function and by all means does return a promise as per ES6 standards.

async function getAuthenticationToken() {
    try {
        const tokenUri = `${keycloakAuthServerUrl}realms/${keycloakRealm}/protocol/openid-connect/token`;

        const params = new URLSearchParams();
        params.append('client_id', DB_KEYCLOAK_CLIENT_ID);
        params.append('client_secret', DB_KEYCLOAK_CLIENT_SECRET);
        params.append('grant_type', MY_GRANT_TYPE);

        const { data } = await axios.post(
            tokenUri,
            params,
            {
                headers: {
                    'Content-Type': 'application/x-www-form-urlencoded'
                }
            }
        );
        return data.access_token;
    } catch (err) {
        logger.error('[getAuthenticationToken] Error getting authentication token', err);
        throw err;
    }
}

async function insertToMd4DatabaseApi(dbTable, data) {
    logger.info('[insertToMd4DatabaseApi] Insert data to Database API', { dbTable });
    logger.debug('[insertToMd4DatabaseApi] Data being saved', { dbTable, data });
    try {
        const authToken = await getAuthenticationToken(); // This part gets underlined by Sonar lint
        return axios.post(`/${dbTable}`, data, {
            baseURL: apiBaseUri,
            headers: {
                'some-auth-stuff': `Bearer ${authToken}`
            }
        });
    } catch (err) {
        logger.error('[insertToMd4DatabaseApi] Error saving data to Database API', err);
        throw err;
    }
}

This rule is causing some stir in my code and has caused some bugs. Please add support for async functions in JavaScript as this is a bug in linting at the moment.

[vilchik-elena]

Hi @peterpoliwoda,

It seems I rushed to add the ticket to the milestone as I can't reproduce it. Could you share self contained code snippet reproducing FP?

[peterpoliwoda]

Hi @vilchik-elena I've updated the code above to reflect the two actual functions in case the code wasn't showing up for you. I updated to show that my underlined code is also wrapped within a try/catch block which might be the reason it's not handled. Please try now. The underlined part is the one with the comment // This part gets underlined

[vilchik-elena]

I still fail to reproduce, even with axios installed. Are you using latest plugin?
Could you may be try to set up a separate project (so that dependencies are set)?

[peterpoliwoda]

I found there was a single quote after MY_GRANT_TYPE' which shouldn't be there. Code is fixed above.

We are both speaking about the SonarLint VSCode Plugin BTW, right?

I've set up a project with a single file where it also gives out here:
https://github.com/peterpoliwoda/test-sonarlint

[search-acumen]

Same issue here using SonarLint VSCode Plugin 2.0.0

[vilchik-elena]

thanks for reproducer! having it on my machine as well, investigating why I can't reproduce the same in unit test

[search-acumen]

If it helps, it seems to only happen if the async function is in the same js file as the await. Awaits to imported async functions don't seem to throw the warning.

[codyebberson]

Here is another example, no dependencies, only 8 lines:

interface MyQuery<T> extends Pick<Promise<T>, keyof Promise<T>> {
  toQuery(): string;
}

export async function demo(query: MyQuery<string>) {
  const result = await query;
  console.log(result);
}

This is effectively what Knex is doing, which is a pretty popular SQL builder:

https://github.com/knex/knex/blob/dfbc76b915fff9f40e806ac2e0aafe8e4e5b6965/types/index.d.ts#L1742

interface ChainableInterface<T = any> extends Pick<Promise<T>, keyof Promise<T> & ExposedPromiseKeys>, StringTagSupport {
  generateDdlCommands(): Promise<{ pre: string[], sql: string[], post: string[] }>;
  toQuery(): string;

[vilchik-elena]

It seems related to the way we do analysis of JavaScript files using TypeScript compiler. Hope to fix it soon

@codyebberson Yet it seems that you have another problem, clearly you are using TypeScript itself and with some type Pick I don't know of. Could you create another ticket describing shortly what this type is doing?

[vilchik-elena]

Finally found the root problem, we are missing type information because in SonarLint context we are not able to generate tsconfig file (

SonarJS/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/eslint/TsConfigProvider.java

Lines 141 to 145 in 1b593bd

	if (context.runtime().getProduct() == SonarProduct.SONARLINT) {
	// we don't support per analysis temporary files in SonarLint see https://jira.sonarsource.com/browse/SLCORE-235
	LOG.warn("Generating temporary tsconfig is not supported in SonarLint context.");
	return emptyList();
	}

).

We want to do a major rework so that this generated tsconfig file would not be required but until that we can provide a simple fix to not report if type of awaited argument is unknown.

[peterpoliwoda]

Wow, that's completely unrelated then hah. Of course that was the cause, what else haha. Thank you for looking into that Elena 🐺 !

[Tranthanh98]

I have a same issue, my function obviously return a Promise but sonarqube show this error

[tr3ysmith]

Same issue here, is there something we have to update in VSCode to make this fix active?

[vilchik-elena]

@tr3ysmith yeah. If you are not using connected mode, then update SonarLint plugin

[kamesh95]

@vilchik-elena We are using SonarQube Enterprise v8.9.0 and still this issue shows up on our dashboard as a Code Smell. Can you let me know with which version of SonarQube this false positive is not detected? Thanks.

[vilchik-elena]

@kamesh95 you should update to latest SQ (9.2 as of now)

[robertpatrick]

This issue is a pain for our JavaScript project. Basically, any async function call always returns a Promise--even if the code does not indicate it. I have to turn this rule off because its implementation is naive and results in thousands of false positives...

[wbt]

Would it be possible to get this backported into an LTS release? I'm seeing it actively cause the introduction of runtime-crash-causing bugs in code.

[vishalamrutiyacodemonk]

Which version of SQ resolves this issue? Currently, I installed 9.6.1 version

[kamal-brill]

We still see it even in "Version 9.9.4 (build 87374)". Is there any update on this ?