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.

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

Bug risk in : async function should have await expression #270

Open
philipjonsen opened this issue Sep 13, 2023 · 0 comments
Open

Bug risk in : async function should have await expression #270

philipjonsen opened this issue Sep 13, 2023 · 0 comments

Comments

@philipjonsen
Copy link

DESCRIPTION

A function that does not contain any await expressions should not be async (except for some edge cases in TypeScript which are discussed below). Asynchronous functions in JavaScript behave differently than other functions in two important ways:

The return value is always a Promise.
You can use the await operator inside them.
Functions are made async so that we can use the await operator inside them. Consider this example:

async function fetchData(processDataItem) {
const response = await fetch(DATA_URL);
const data = await response.json();

return data.map(processDataItem);

}
Asynchronous functions that don't use await might be an unintentional result of refactoring.

Note: This issue ignores async generator functions. Generators yield rather than return a value and async generators might yield all the values of another async generator without ever actually needing to use await.

In TypeScript, one might feel the need to make a function async to comply with type signatures defined by an interface. Ideally, the code should be refactored to get rid of such restrictions, but sometimes that isn't feasible (For example, when we are implementing an interface defined in a 3rd party library like Next.js).

This situation can easily be circumvented by returning the value with a call to Promise.resolve:

interface HasAsyncFunc {
getNum: () => Promise
}

// Not recommended:
const o: HasAsyncFunc = {
async getNum() { return 1 }
}

// Recommended:
const o: HasAsyncFunc = {
// We only use Promise.resolve to adhere to the type
// of the surrounding object.
getNum() { return Promise.resolve(1) }
}
It is also advised to add a comment near the redundant promise to make the intent clear.

BAD PRACTICE

async function fetchData(): string {
// readFileSync is a synchronous function that blocks
// the main thread, and thus does not need to be awaited
return fs.readFileSync("data.txt", "utf-8");
}

performAction(async () => { console.log("no awaits in here") });

RECOMMENDED

async function fetchDataAsync(): Promise {
return await fs.readFile("data.txt", "utf-8")
}

performAction(async () => { await writeToFile(data) });

// Allow empty functions.
async function no_op() {}

Look here to fix it:

Found async function without any await expressions

src/api.ts

  • @param results
  • @param encodeData
    */
    export const retryCalls = async (
    provider: ProviderLike,
    addresses: string | string[],
    contracts: string | string[],
    results: Result[],
    encodeData: (address: string) => string
    ): Promise<Result[]> => {
    return Promise.all(
    results.map(async (result, index) => {
    if (result[0]) {
    return result;
    }
    const address = typeof addresses === 'string' ? addresses : addresses[index];
    const contractAddress = typeof contracts === 'string' ? contracts : contracts[index];
    const data = encodeData(address);
    try {
    const newResult = await call(provider, contractAddress, data);
    return [true, newResult] as [boolean, Uint8Array];
    } catch {
    // noop
    }
    return result;
    })
    );
    };

Found async function without any await expressions

src/providers/eip-1193.ts

return (provider as EIP1193ProviderLike)?.request !== undefined;

},

send: async (provider: EIP1193ProviderLike, method: string, params: unknown[]): Promise => {
const payload = getPayload(method, params);
return provider.request(payload);
}
};

export default provider;

Found async function without any await expressions

src/providers/provider.test.ts

jest.mock('./eip-1193', () => ({
isProvider: jest.fn(),
send: jest.fn().mockImplementation(async () => '0x00')
}));

jest.mock('./ethers', () => ({
Found async function without any await expressions
src/providers/provider.test.ts

jest.mock('./ethers', () => ({
isProvider: jest.fn(),
send: jest.fn().mockImplementation(async () => '0x00')
}));

jest.mock('./http', () => ({
Found async function without any await expressions
src/providers/provider.test.ts

jest.mock('./http', () => ({
isProvider: jest.fn(),
send: jest.fn().mockImplementation(async () => '0x00')
}));

jest.mock('./web3', () => ({

1
2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant