openapi-typescript-codegen icon indicating copy to clipboard operation
openapi-typescript-codegen copied to clipboard

CancelablePromise fails to resolve with Next.js 13

Open pitmullerIngka opened this issue 2 years ago • 22 comments

Describe the bug I've tried to upgrade an application from Next.js 12 (v12.3.4) to 13 (v13.4.8) and suddenly experienced issues when fetching data from the API through the generated typescript client. It still send out the requests and I got a response from the API, but the Promise would not resolve and just stay stuck there. I did not see or receive error messages, but even after waiting minutes, there would never be a response.

Going into the generated code, it seemed to fail resolving in CancelablePromise.ts in the constructor trying to call the resolve function on the Promise. I troubleshooted with some console.logging to debug the issue, like the following (CancelablePromise.ts:40-50):

this.#promise = new Promise<T>((resolve, reject) => {
      this.#resolve = resolve;
      this.#reject = reject;

      const onResolve = (value: T | PromiseLike<T>): void => {
        console.log("starting to resolve");
        if (this.#isResolved || this.#isRejected || this.#isCancelled) {
          return;
        }
        console.log("resolving");
        this.#isResolved = true;
        this.#resolve?.(value);
        console.log("resolved");
      };

The console logs:

13:57:08.464 starting to resolve [CancelablePromise.ts:45:16]
13:57:08.465 resolving [CancelablePromise.ts:49:16]

This issue also seems to occur for reject.

OS: Linux Mint 21.1 Cinnamon Browser: Firefox 114.0.2

Tested also on a Mac with Chromium, which showed an error message on row 56:

this.#reject?.(reason);
TypeError: (intermediate value)(intermediate value)(intermediate value) is not a function

Now, I have no clue why this issue occurs when updating Next.js for me, but something in the background must've changed that I'm not aware of. The exact same generated code works for me with Next.js 12. It seems to try to call the resolve() function before the instance is initiated and not resolving the valid syntax of this.#resolve?.(value) correctly.

As said, no clue why this happens, but doing the undefined check without an intermediate value, seems to resolve this issue for me, so I would propose this in a PR and would be interested in your opinions on this.

So instead of this.#resolve?.(value);, I propose:

if (this.#resolve) this.#resolve(value);

pitmullerIngka avatar Jul 05 '23 12:07 pitmullerIngka

I've been running into the very same issue, except that I'm not using Next.js. I'm using generated code inside a custom written plugin for backstage. I was not able to pin it down exactly yet, but in my case it fails after upgrading the package @backstage/backend-tasks from v0.5.2 to v0.5.3.

The change proposed by @pitmullerIngka does fix the issue for me as well.

moee avatar Jul 07 '23 08:07 moee

The change proposed by @pitmullerIngka does fix the issue for me as well.

I've had the same situation @pitmullerIngka with nextjs13. I've adopted the PR's changes manually it solves problems.

tomohirohiratsuka avatar Jul 11 '23 08:07 tomohirohiratsuka

I wonder if this could be an issue with SWC? @pitmullerIngka could you try to take different canaries between 13.4.7 and 13.4.8? It seems like that the issue might have been introduced in that span of releases.

More precisely, between v13.4.8-canary.9 and v13.4.8-canary.10

icyJoseph avatar Jul 11 '23 19:07 icyJoseph

@brunouber has confirmed that when upgrading from v13.4.8-canary.9 to v13.4.8-canary.10, the issue starts to show up

  • https://github.com/vercel/next.js/discussions/52561#discussioncomment-6419866

And there was an SWC update: https://github.com/vercel/next.js/commit/484bdebc2468e14a677f71f18457722ba33a69c3

A bit early still, but between canary.9 and canary.10 there was a handful of commits, one of which is an SWC bump.

icyJoseph avatar Jul 11 '23 19:07 icyJoseph

And now, the uphill code battle to generate a small reproduction setup 🤔

icyJoseph avatar Jul 11 '23 19:07 icyJoseph

Yes I can confirm, that this issue is introduced in v13.4.8-canary.10

pitmullerIngka avatar Jul 12 '23 05:07 pitmullerIngka

It looks like building an failing example is not very trivial... Maybe if we copy the CancelablePromise code into a Next.js project, and try to await the promise there?

What is it exactly not working with this promise extension?

icyJoseph avatar Jul 12 '23 06:07 icyJoseph

Sounds sound.

https://github.com/ferdikoomen/openapi-typescript-codegen/blob/056b1c7b87c556b686fe1dde102e93ac5885e474/src/templates/core/CancelablePromise.hbs#L47-L61

Here in line 52 and 60 it's not resolving the intermediate value of this.#resolve?/this.#reject? correctly, because it tries to call it as function, even if it's undefined and then throws:

TypeError: (intermediate value)(intermediate value)(intermediate value) is not a function

So I'd guess it has mostly to do with handling of intermediate values. But I have no clue how that might be affected by Next.js or SWC or how these intermediate values are evaluated in ECMAScript

pitmullerIngka avatar Jul 13 '23 05:07 pitmullerIngka

Elaborating further on @pitmullerIngka 's comment, this is a simple case that allows to reproduce the error when transpiled with swc:

  • "@swc/cli": "^0.1.62",
  • "@swc/core": "^1.3.69"
const MAIN = async () => {
  console.log("Running test");

  const p = new CancelablePromise((r) => {
    setTimeout(r, 1000, 42);
  });

  await p;

  console.log(p);
};

MAIN();

CancelPromise is defined as:

class CancelError extends Error {
  constructor(message: string) {
    super(message);
    this.name = "CancelError";
  }

  public get isCancelled(): boolean {
    return true;
  }
}

interface OnCancel {
  readonly isResolved: boolean;
  readonly isRejected: boolean;
  readonly isCancelled: boolean;

  (cancelHandler: () => void): void;
}

class CancelablePromise<T> implements Promise<T> {
  #isResolved: boolean;
  #isRejected: boolean;
  #isCancelled: boolean;
  readonly #cancelHandlers: (() => void)[];
  readonly #promise: Promise<T>;
  #resolve?: (value: T | PromiseLike<T>) => void;
  #reject?: (reason?: any) => void;

  constructor(
    executor: (
      resolve: (value: T | PromiseLike<T>) => void,
      reject: (reason?: any) => void,
      onCancel: OnCancel
    ) => void
  ) {
    this.#isResolved = false;
    this.#isRejected = false;
    this.#isCancelled = false;
    this.#cancelHandlers = [];
    this.#promise = new Promise<T>((resolve, reject) => {
      this.#resolve = resolve;
      this.#reject = reject;

      const onResolve = (value: T | PromiseLike<T>): void => {
        if (this.#isResolved || this.#isRejected || this.#isCancelled) {
          return;
        }
        this.#isResolved = true;
        this.#resolve?.(value);
      };

      const onReject = (reason?: any): void => {
        if (this.#isResolved || this.#isRejected || this.#isCancelled) {
          return;
        }
        this.#isRejected = true;
        this.#reject?.(reason);
      };

      const onCancel = (cancelHandler: () => void): void => {
        if (this.#isResolved || this.#isRejected || this.#isCancelled) {
          return;
        }
        this.#cancelHandlers.push(cancelHandler);
      };

      Object.defineProperty(onCancel, "isResolved", {
        get: (): boolean => this.#isResolved,
      });

      Object.defineProperty(onCancel, "isRejected", {
        get: (): boolean => this.#isRejected,
      });

      Object.defineProperty(onCancel, "isCancelled", {
        get: (): boolean => this.#isCancelled,
      });

      return executor(onResolve, onReject, onCancel as OnCancel);
    });
  }

  get [Symbol.toStringTag]() {
    return "Cancellable Promise";
  }

  public then<TResult1 = T, TResult2 = never>(
    onFulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | null,
    onRejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | null
  ): Promise<TResult1 | TResult2> {
    return this.#promise.then(onFulfilled, onRejected);
  }

  public catch<TResult = never>(
    onRejected?: ((reason: any) => TResult | PromiseLike<TResult>) | null
  ): Promise<T | TResult> {
    return this.#promise.catch(onRejected);
  }

  public finally(onFinally?: (() => void) | null): Promise<T> {
    return this.#promise.finally(onFinally);
  }

  public cancel(): void {
    if (this.#isResolved || this.#isRejected || this.#isCancelled) {
      return;
    }
    this.#isCancelled = true;
    if (this.#cancelHandlers.length) {
      try {
        for (const cancelHandler of this.#cancelHandlers) {
          cancelHandler();
        }
      } catch (error) {
        console.warn("Cancellation threw an error", error);
        return;
      }
    }
    this.#cancelHandlers.length = 0;
    this.#reject?.(new CancelError("Request aborted"));
  }

  public get isCancelled(): boolean {
    return this.#isCancelled;
  }
}

brunouber avatar Jul 13 '23 07:07 brunouber

@swc/[email protected] is the first version for which I can reproduce the error; I think we can close the issue here, and continue discussing this on SVC repo.

brunouber avatar Jul 13 '23 07:07 brunouber

Awesome investigation @brunouber @pitmullerIngka - yeah if we have code and a version that breaks we should go to the SWC playground and create a shareable link. Use that when creating the SWC bug.

icyJoseph avatar Jul 13 '23 07:07 icyJoseph

Really nice work @brunouber ! Let's hope they can fix it in due time in SWC directly, so we can skip the weird fix here.

pitmullerIngka avatar Jul 14 '23 06:07 pitmullerIngka

In the meantime, going to refactor private properties after generating the client with babel:

import { CodeGenerator } from "@babel/generator";
import { parse } from "@babel/parser";
import traverse from "@babel/traverse";
import * as t from "@babel/types";
import fs from "fs/promises";
import path from "path";

export const patchCancellablePromise = async () => {
  const src = path.resolve(process.cwd(), "$CLIENT_PATH/core/CancelablePromise.ts");

  let content = await fs.readFile(src, { encoding: "utf-8" });
  content = content.replaceAll("#", "");

  const ast = parse(content, { sourceType: "module", plugins: ["typescript"] });
  traverse(ast, {
    ClassMethod: (current) => {
      if (t.isIdentifier(current.node.key, { name: "isCancelled" })) {
        current.remove();
        current.skip();
      }
    },
  });

  const { code } = new CodeGenerator(ast).generate();
  await fs.writeFile(src, code, "utf-8");
};

grikomsn avatar Aug 11 '23 18:08 grikomsn

@grikomsn thanks for sharing this. I am still new to JS land, probably stupid question but how would I call this?

I see you are exporting the patchCancellablePromise so could I execute this in my package.json ? Would love some hints, as I am stuck converting FastAPI Open API Speck to some typescript fetch client.

feliche93 avatar Aug 27 '23 21:08 feliche93

I think I'm facing the same issue on Next.js 13.5. The CancelablePromise class no longer works, such that all calls sit there hanging without resolving. Swapping CancelablePromise for Promise fixes it as a workaround, at least. I'll try this AST workaround. Is there a more official solution?

nandorojo avatar Oct 24 '23 17:10 nandorojo

@grikomsn's workaround did it for me, thanks! https://github.com/ferdikoomen/openapi-typescript-codegen/issues/1626#issuecomment-1675216771

nandorojo avatar Oct 24 '23 17:10 nandorojo

@grikomsn thanks for sharing this. I am still new to JS land, probably stupid question but how would I call this?

I see you are exporting the patchCancellablePromise so could I execute this in my package.json ? Would love some hints, as I am stuck converting FastAPI Open API Speck to some typescript fetch client.

Reasonable request. Basically, you'd put everything in a Node.js file to run, instead of using the CLI.

In my case I used TypeScript, so ts-node.

First I created this file (fix-cancelable-promise.ts):

Click to view the code
// https://github.com/ferdikoomen/openapi-typescript-codegen/issues/1626#issuecomment-1675216771
import { CodeGenerator } from '@babel/generator'
import { parse } from '@babel/parser'
import traverse from '@babel/traverse'
import * as t from '@babel/types'
import * as fs from 'fs/promises'
import * as path from 'path'

export const fixCancellablePromise = async (dir: string) => {
  const src = path.resolve(process.cwd(), dir, 'core/CancelablePromise.ts')

  let content = await fs.readFile(src, { encoding: 'utf-8' })
  content = content.replaceAll('#', '')

  const ast = parse(content, {
    sourceType: 'module',
    plugins: ['typescript'],
  }) as any
  traverse(ast, {
    ClassMethod: (current) => {
      if (t.isIdentifier(current.node.key, { name: 'isCancelled' })) {
        current.remove()
        current.skip()
      }
    },
  })

  const { code } = new CodeGenerator(ast).generate()
  await fs.writeFile(src, code, 'utf-8')
}

Next make another file (like codegen.ts):

#!/usr/bin/env node

import * as openapi from 'openapi-typescript-codegen'
import * as path from 'path'
import * as fs from 'fs'
import { fixCancellablePromise } from './fix-cancelable-promise'

const run = async () => {
  const output = 'path/to/output/folder'
  await openapi.generate({
    // edit the other inputs here, just like the CLI
    input: `https://your-url.com/openapi.json`, // update this to match yours
    output,
  })

  fixCancelablePromise(output) // override the broken 
}

run()

Lastly, call the file:

npx ts-node codegen.ts

If you don't want to use TypeScript you can write // @ts-nocheck at the top of the files if you get any warnings. Or you can rewrite it to use Node directly (a simple copy-paste into ChatGPT telling it to convert this code to pure Node.js will work). Then you can just run node codegen.js instead of npx ts-node codegen.ts.

nandorojo avatar Oct 24 '23 18:10 nandorojo

The related issue in swc was fixed today and it looks like it will be part of the next release. 🎉

fspoettel avatar Nov 03 '23 09:11 fspoettel

Update: i've upgraded to Next 14.0.3 and it seems like the bug is fixed there. Can anyone please confirm?

Lamarcke avatar Nov 27 '23 02:11 Lamarcke

I've upgraded to Next 14.0.4 and the bug is still :-/

ntvinhit avatar Dec 28 '23 14:12 ntvinhit

14.1.1 has this error, 14.1.2 solves it.

lucaslosi avatar Mar 06 '24 04:03 lucaslosi

IMH, the bug hasn't happened ever since ~14.0.3, please try to upgrade your NextJS and test it

Lamarcke avatar Mar 06 '24 11:03 Lamarcke