Memory Leak Due to Response Cloning

[maiky-apex]

Description

We have identified a memory leak in our application that appears to be related to the swagger-typescript-api package. After thorough investigation, we traced the issue to a change introduced in this commit, which was merged on September 5, 2024.

Problem

In the commit referenced above, the response of the auto-generated customFetch function is cloned before being returned. This clone operation is causing a memory leak in our application.

Observation

Without the clone operation, the memory leak does not occur.
The memory usage increases steadily when the clone method is used, indicating a potential problem with how the response is handled.

Steps to Reproduce

Use the swagger-typescript-api package version that includes the commit ac99885.
Generate API client code using swagger-typescript-api.
Make API requests using the generated client.
Observe memory usage over time with and without the response being cloned.

Expected Behavior

The response should be returned without causing a memory leak, and the memory usage should remain stable.

Actual Behavior

The memory usage increases steadily, leading to a memory leak when the response is cloned.

Additional Context

Here's a snippet of the auto-generated code that causes the issue:

    return this.customFetch(`${baseUrl || this.baseUrl || ''}${path}${queryString ? `?${queryString}` : ''}`, {
      ...requestParams,
      headers: {
        ...(requestParams.headers || {}),
        ...(type && type !== ContentType.FormData ? { 'Content-Type': type } : {}),
      },
      signal: (cancelToken ? this.createAbortSignal(cancelToken) : requestParams.signal) || null,
      body: typeof body === 'undefined' || body === null ? null : payloadFormatter(body),
    }).then(async (response) => {
      const r = response.clone() as HttpResponse<T, E>;
      r.data = null as unknown as T;
      r.error = null as unknown as E;

      const data = !responseFormat
        ? r
        : await response[responseFormat]()
            .then((data) => {
              if (r.ok) {
                r.data = data;
              } else {
                r.error = data;
              }
              return r;
            })
            .catch((e) => {
              r.error = e;
              return r;
            });

      if (cancelToken) {
        this.abortControllers.delete(cancelToken);
      }

      if (!response.ok) throw data;
      return data;
    });
  };
}

[smorimoto]

@maiky-apex This seems correct, but I don't have enough time to work on this, so could you raise a PR?

[maiky-apex]

@smorimoto
Thank you for the quick response. I understand the importance of the response cloning to address the issue mentioned in PR #670 related to node-fetch issue #533.

While I have confirmed that removing the clone operation resolves the memory leak, I don't want to remove it outright without considering the implications for the previously addressed issue.

Unfortunately, I also don't have enough time at the moment to work on a proper solution that addresses both issues. I will try to work on it soon, but in the meantime, if someone else sees this comment and has a potential solution, please feel free to open a PR.

[smorimoto]

Thanks a lot @maiky-apex

[smorimoto]

@maiky-apex @depsimon I don't know how different the node-fetch implementation is from the standard fetch implementation, but keeping in mind that we are now using native fetch, we may also need to verify that the commit is really necessary.

[cwd-nial]

Any update on this issue? We currently have to remove the .clone() part every time we rebuild our api-client. I am wondering why this issue is not being addressed, as in our case it was causing a severe memory leak...

[ETOPS7]

🆙

[FDiskas]

In latest version we still have clone here

const r = response.clone() as HttpResponse<T, E>;