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][typescript-fetch] New FetchError class incorrectly extends base class Error #12927

Closed
5 tasks done
sevein opened this issue Jul 19, 2022 · 5 comments
Closed
5 tasks done

Comments

@sevein
Copy link

sevein commented Jul 19, 2022

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
Description

3a2bbbb introduced a new class (FetchError) that produces a build issue in TypeScript when using the strict mode:

src/openapi-generator/runtime.ts:239:14 - error TS2415: Class 'FetchError' incorrectly extends base class 'Error'.
  Types of property 'cause' are incompatible.
    Type 'unknown' is not assignable to type 'Error | undefined'.

239 export class FetchError extends Error {
                 ~~~~~~~~~~

Example of FetchError:

export class FetchError extends Error {
name: "FetchError" = "FetchError";
constructor(public cause: unknown, msg?: string) {
super(msg);
}
}

openapi-generator version

v6.0.1 (and master)

OpenAPI declaration file content or url

N/A

Generation Details

N/A

Steps to reproduce

N/A

Related issues/PRs

FetchError was introduced in the context of this issue: #12716.

Suggest a fix

N/A

@sevein
Copy link
Author

sevein commented Jul 20, 2022

Hey @orange-buffalo, since you designed FetchError I was wondering if you can think of a solution. Thank you in advance!

@ssuchkov
Copy link

I had this very issue because I had "esnext" lib in tsconfig.json
Since lib.es2022.error.d.ts brings interface Error {cause?: Error} it considered to be overridden in incorrect way.

Removing "esnext" fixed the issue for now.

@orange-buffalo
Copy link
Contributor

Th current code for FetchError is the following:

export class FetchError extends Error {
    name: "FetchError" = "FetchError";
    constructor(public cause: unknown, msg?: string) {
        super(msg);
    }
}

As far as I can see, it is compliant with ES5 definition:

interface Error {
    name: string;
    message: string;
    stack?: string;
}

(probably this is the reason I missed it during implementation, as I transpile to ES5).

However, in newer versions the definition changes:

interface Error {
    cause?: Error;
}

Which seems to be clashing with FetchError.cause.

My suggestion would be to align FetchError with latest ES. As the very minimum, cause should be changed from unknown to Error. Maybe, the order of parameters could be also changed to be in-sync with new Error constructor: new (message?: string, options?: ErrorOptions): Error.
Formally it breaks backwards compatibility, since FetchError is a public class. One could argue that this is fine since we are fixing a bug in the first place.

CC @macjohnny

@macjohnny
Copy link
Member

@orange-buffalo can you file a PR?

@orange-buffalo
Copy link
Contributor

#13004. I decided to go with the minimal changes. Checked the samples build with es5 and esnext, both passed.

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

No branches or pull requests

4 participants