klippa-app/nativescript-http

NS8 Webpack path replacement module does not take into account Windows 10 backslashes

manciuszz opened this issue · 9 comments

... therefore for Windows 10 users the webpack module will always fail.

// ..\@klippa\nativescript-http\webpack\index.js
apply(compiler) {
        const resourceRegExp = /http-request|image-cache/;
        compiler.hooks.normalModuleFactory.tap(
            "NativeScriptHTTPPlugin",
            nmf => {
                nmf.hooks.beforeResolve.tap("NativeScriptHTTPPlugin", result => {
                    if (!result) return;

                    // Replace http-request imports by our own.
                    if (resourceRegExp.test(result.request)) {
                        // Replace the relative http-request import from core http.
                        if (this.replaceHTTP && result.request === "./http-request") {
                            console.log("[@klippa\nativescript-http\webpack\index.js]", result.context, result.context.endsWith("@nativescript/core/http"), result.context.endsWith("@nativescript\\core\\http")); 
                            // Result: [@klippa\nativescript-http\webpack\index.js] D:\Folder1\Folder2\node_modules\@nativescript\core\http false true

                            if (result.context.endsWith("@nativescript/core/http")) {
                                result.request = "../../../@klippa/nativescript-http";
                            }
                            if (result.context.endsWith("tns-core-modules/http")) {
                                result.request = "../../@klippa/nativescript-http";
                            }
                        }

                        // Replace the relative http-request import from core image-cache (for iOS).
                        if (this.replaceHTTP && result.request === "../../http/http-request") {
                            if (result.context.endsWith("@nativescript/core/ui/image-cache")) {
                                result.request = "../../../../@klippa/nativescript-http";
                            }
                            if (result.context.endsWith("tns-core-modules/ui/image-cache")) {
                                result.request = "../../../@klippa/nativescript-http";
                            }
                        }

                        // Replace the relative image-cache import from core ui.
                        if (this.replaceImageCache && result.request === "./image-cache") {
                            if (result.context.endsWith("@nativescript/core/ui")) {
                                result.request = "../../../@klippa/nativescript-http/image-cache";
                            }
                            if (result.context.endsWith("tns-core-modules/ui")) {
                                result.request = "../../@klippa/nativescript-http/image-cache";
                            }
                        }

                        // When other code directly imports http-request.
                        if (this.replaceHTTP && (result.request === "@nativescript/core/http/http-request" || result.request === "tns-core-modules/http/http-request")) {
                            result.request = "@klippa/nativescript-http";
                        }

                        // When other code directly imports image-cache.
                        if (this.replaceImageCache && (result.request === "@nativescript/core/ui/image-cache" || result.request === "tns-core-modules/image-cache")) {
                            // Make sure we don't ruin our own import.
                            // We import image-cache for iOS because that implementation is fine.
                            if (!result.context.endsWith("src/image-cache") && !(result.context.endsWith("@klippa/nativescript-http/image-cache"))) {
                                result.request = "@klippa/nativescript-http/image-cache";
                            }
                        }
                    }
                    return;
                });
            }
        );
    }

Solution should be obvious.

EDIT:

In addition, I encountered an issue where the built-in "parseJSON" function got triggered when serializing console.log output of an object and the response content was an empty string, so it crashed the app. I realize the problem is not inside the 'parseJSON' itself, but in the way nativescript console.log works? Regardless, I think an empty passed string input to 'parseJSON' should return an empty string instead of continuing to call JSON.parse("").

P.S ... was too lazy to open a new issue, so mentioned it here.

I'll check for Windows support, shouldn't be too hard.

About the JSON Parse, sending an empty string to JSON.parse should not crash your app, it should just result in an error "JS: ERROR SyntaxError: Unexpected end of JSON input".

Are you sure there wasn't something else that caused this? I think console.log() triggers a toString() on the content and that is causing some issues. I'll try to see if I can reproduce it.

Ok, I found out the crash:
console.log calls toString on the result, the toString method on the result is this:

                let str: string;
                if (encoding) {
                    str = decodeResponse(result.raw, encoding);
                } else if (result.responseAsString) {
                    str = result.responseAsString;
                }

                if (typeof str === "string") {
                    return str;
                } else {
                    throw new Error("Response content may not be converted to string");
                }

Which is the same as in core HTTP. When the result is not a string, it will throw the error, and console.log does not like it. So it's a bug in console.log implementation of NS. I will open a bug in core.

I'll check for Windows support, shouldn't be too hard.

a simple one liner result.context = result.context.replace(/\\/g, '/'); is enough to fix the issue.

About the JSON Parse, sending an empty string to JSON.parse should not crash your app, it should just result in an error "JS: ERROR SyntaxError: Unexpected end of JSON input".

That's exactly the error I got, which triggered a '.catch' on a promise when sending a http request which at that point in time still had some broken code that lead to an app crash.

Are you sure there wasn't something else that caused this? I think console.log() triggers a toString() on the content and that is causing some issues. I'll try to see if I can reproduce it.

Yea, like I thought - it is a bug with 'console.log'. Anyways, a quick try/catch for the win! xD

Is it possible that you were trying to get the result as json? Because a console.log would not trigger that.

However, I did find a bug in console.log in NS, when you have an object that implements toString(), and you console.log that object, and the toString throws an error, the whole app crashes, so maybe something like that happened to you? I will open a bug report soon.

For the Windows paths you can probably just use path.sep from nodejs as path separator instead of hardcoded slash. I will add that soon.

Is it possible that you were trying to get the result as json? Because a console.log would not trigger that.

deleteRequest: (resource, params = {}) =>
            httpClient(resource + generateParamQuery(params.query, "?"), {
                ...params,
                method: "DELETE",
            }).then((response) => {
                const result = response;
                try { console.log(TAG, "[fetchDelete-Response]", resource, result); } catch {} // NOTE: This line is haunted. For whatever magic reason this line calls .toJSON() when stringifying the output and the JSON input happens to be an empty string.
                return result.ok ? result : Promise.reject(result);
            }).catch((e) => {
                console.log(TAG, "[fetchDelete-Error]", resource + generateParamQuery(params.query, "?"), e);
                throw(e);
            }
        ),

 // ... after the 'deleteRequest' was sent, the returned 'response' is an object that contains a function 'json: () => Promise.resolve(response.content.toJSON())'

Here's a debugger screenshot without try/catch:
chrome_xZiKiFBUy7

chrome_FuCIYNyel2

Spooky.

Can you tell me which http client you are using? I don't really think this is something we should solve in this plugin.

httpClient is basically just a 'fetch' wrapper that uses 'nativescript-http' Http.request method.

import { Http } from "@nativescript/core";

export const fetch = (url, options = {}) => { // fetch adapter for 'Http.request'
    if (url instanceof Request) {
        options = (({ mode, headers, method, _bodyText }) => ({ mode, method, headers: headers.map, body: _bodyText }))(url);
        url = url.url;
    }

    options.url = url;
    options.timeout = options.timeout || 3000;
    options.content = options.content || options.body;
    return Http.request(options).then((response) => {
        response.status = response.statusCode;
        response.ok = response.statusCode >= 200 && response.statusCode < 300;
        response.json = () => Promise.resolve(response.content.toJSON());
        return response;
    });
};

Anyways, this 'console.log' issue is just something I randomly encountered after installing 'nativescript-http' plugin. It's baffling, sure, but quite easy to handle.

I have just done a test. When you console.log response, it does not execute the json method, and thus also does not result in the given error.

Fixed with #52