Code cleanup, deprecation notice, add maxRedirects

preload/src/api/fetch.js

@@ -1,8 +1,23 @@
 import * as https from "https";
 import * as http from "http";
 
+const MAX_DEFAULT_REDIRECTS = 20;
 const redirectCodes = new Set([301, 302, 307, 308]);
 
+/**
+ * @typedef {Object} FetchOptions
+ * @property {"GET" | "PUT" | "POST" | "DELETE"} [method] - Request method.
+ * @property {Record<string, string>} [headers] - Request headers.
+ * @property {"manual" | "follow"} [redirect] - Whether to follow redirects.
+ * @property {number} [maxRedirects] - Maximum amount of redirects to be followed.
+ * @property {AbortSignal} [signal] - Signal to abruptly cancel the request
+ * @property {Uint8Array | string} [body] - Defines a request body. Data must be serializable. 
+ */
+
+/**
+ * @param {string} url
+ * @param {FetchOptions} options
+ */
 export function nativeFetch(url, options) {
     let state = "PENDING";
     const data = {content: [], headers: null, statusCode: null, url: url, statusText: "", redirected: false};
@@ -10,7 +25,7 @@ export function nativeFetch(url, options) {
     const errors = new Set();
 
     /** * @param {URL} url */
-    const execute = (url, options, redirect = false) => {
+    const execute = (url, options, redirectCount = 0) => {
         const Module = url.protocol === "http" ? http : https;
         
         const req = Module.request(url.href, {
@@ -18,13 +33,31 @@ export function nativeFetch(url, options) {
             method: options.method ?? "GET"
         }, res => {
             if (redirectCodes.has(res.statusCode) && res.headers.location && options.redirect !== "manual") {
-                const final = new URL(res.headers.location);
+                redirectCount++;
+
+                if (redirectCount >= (options.maxRedirects ?? MAX_DEFAULT_REDIRECTS)) {
+                    state = "ABORTED";
+                    const error = new Error(`Maximum amount of redirects reached (${options.maxRedirects ?? MAX_DEFAULT_REDIRECTS})`);
+                    errors.forEach(e => e(error));
+                    
+                    return;
+                }
+
+                let final;
+                try {
+                    final = new URL(res.headers.location);
+                }
+                catch (error) {
+                    state = "ABORTED";
+                    errors.forEach(e => e(error));
+                    return;
+                }
 
                 for (const [key, value] of new URL(url).searchParams.entries()) {
                     final.searchParams.set(key, value);
                 }
 
-                return execute(final, options, true);
+                return execute(final, options, redirectCount);
             }
 
             res.on("data", chunk => data.content.push(chunk));
@@ -34,7 +67,7 @@ export function nativeFetch(url, options) {
                 data.statusCode = res.statusCode;
                 data.url = url.toString();
                 data.statusText = res.statusMessage;
-                data.redirected = redirect;
+                data.redirected = redirectCount > 0;
                 state = "DONE";
 
                 listeners.forEach(listener => listener());
@@ -50,10 +83,12 @@ export function nativeFetch(url, options) {
             catch (error) {
                 state = "ABORTED";
                 errors.forEach(e => e(error));
-            } finally {
+            }
+            finally {
                 req.end();
             }
-        } else {
+        }
+        else {
             req.end();
         }
 
@@ -65,7 +100,14 @@ export function nativeFetch(url, options) {
         }
     };
 
-    execute(new URL(url), options);
+    try {
+        const parsed = new URL(url);
+        execute(parsed, options);
+    }
+    catch (error) {
+        state = "ABORTED";
+        errors.forEach(e => e(error));
+    }
 
     return {
         onComplete(listener) {

renderer/src/modules/api/fetch.js

@@ -1,5 +1,7 @@
 import Remote from "../../polyfill/remote";
 
+const methods = new Set(["GET" | "PUT" | "POST" | "DELETE"]);
+
 class FetchResponse extends Response {
     constructor(options) {
         super(options.content, {
@@ -30,14 +32,34 @@ const convertSignal = signal => {
     };
 };
 
-export function fetch(url, options = {}) {
+/**
+ * @typedef {Object} FetchOptions
+ * @property {"GET" | "PUT" | "POST" | "DELETE"} [method] - Request method.
+ * @property {Record<string, string>} [headers] - Request headers.
+ * @property {"manual" | "follow"} [redirect] - Whether to follow redirects.
+ * @property {number} [maxRedirects] - Maximum amount of redirects to be followed.
+ * @property {AbortSignal} [signal] - Signal to abruptly cancel the request
+ * @property {Uint8Array | string} [body] - Defines a request body. Data must be serializable. 
+ */
+
+/**
+ * @param {string} url
+ * @param {FetchOptions} options
+ * @returns {Promise<FetchResponse>}
+ */
+export default function fetch(url, options = {}) {
     return new Promise((resolve, reject) => {
-        const ctx = Remote.nativeFetch(url, {
-            ...(options.headers && {headers: options.headers instanceof Headers ? Object.fromEntries(options.headers.entries()) : options.headers}),
-            ...(options.body && {body: options.body}),
-            ...(options.method && {method: options.method}),
-            ...(options.signal && {signal: convertSignal(options.signal)})
-        });
+        const data = {};
+
+        if (typeof options.headers === "object") {
+            data.headers = options.headers instanceof Headers ? Object.fromEntries(options.headers.entries()) : options.headers;
+        }
+
+        if (typeof options.body === "string" || options.body instanceof Uint8Array) data.body = options.body;
+        if (typeof options.method === "string" && methods.has(options.method)) data.method = options.method;
+        if (options.signal instanceof AbortSignal) data.signal = convertSignal(options.signal);
+
+        const ctx = Remote.nativeFetch(url, data);
 
         ctx.onError(error => {
             reject(error);

renderer/src/modules/api/index.js

@@ -12,7 +12,7 @@ import Utils from "./utils";
 import Webpack from "./webpack";
 import * as Legacy from "./legacy";
 import ContextMenu from "./contextmenu";
-import {fetch} from "./fetch";
+import fetch from "./fetch";
 import {DiscordModules} from "modules";
 
 const bounded = new Map();
@@ -58,7 +58,7 @@ export default class BdApi {
     Components = {
         get Tooltip() {return DiscordModules.Tooltip;}
     }
-    fetch = fetch; 
+    Net = {fetch}; 
 }
 
 // Add legacy functions
@@ -128,8 +128,9 @@ BdApi.Components = {
     get Tooltip() {return DiscordModules.Tooltip;}
 };
 
-BdApi.fetch = fetch;
+BdApi.Net = {fetch};
 
 Object.freeze(BdApi);
+Object.freeze(BdApi.Net);
 Object.freeze(BdApi.prototype);
 Object.freeze(BdApi.Components);

renderer/src/polyfill/index.js

@@ -7,6 +7,12 @@ import * as https from "./https";
 import Buffer from "./buffer";
 import crypto from "./crypto";
 import Remote from "./remote";
+import Logger from "common/logger";
+
+const deprecated = new Map([
+    ["request", "Use BdApi.Net.fetch instead."],
+    ["https", "Use BdApi.Net.fetch instead."],
+]);
 
 const originalFs = Object.assign({}, fs);
 originalFs.writeFileSync = (path, data, options) => fs.writeFileSync(path, data, Object.assign({}, options, {originalFs: true}));
@@ -14,6 +20,10 @@ originalFs.writeFile = (path, data, options) => fs.writeFile(path, data, Object.
 
 export const createRequire = function (path) {
     return mod => {
+        if (deprecated.has(mod)) {
+            Logger.warn("Remote~Require", `The "${mod}" module is marked as deprecated. ${deprecated.get(mod)}`);
+        }
+
         switch (mod) {
             case "request": return request;
             case "https": return https;
@@ -42,4 +52,4 @@ require.resolve = (path) => {
     }
 };
 
-export default require;
+export default require;