NativeScript / nativescript-background-http

Background Upload plugin for the NativeScript framework
Apache License 2.0
102 stars 50 forks source link

Does not work with Android 8.0.0? #124

Closed fthuin closed 6 years ago

fthuin commented 6 years ago

Hello,

I have a Sony Xperia X with Android 8.0.0 and I just get an error with I call the function uploadFile(). The code works fine on Samsung S6 Android 7.0 and on iOS.

Note that no request ever arrives to the server so it's not a network error. It does not work on http nor https server.

Here is the portion of code if it helps

import * as bghttpModule from 'nativescript-background-http';

    constructor(
        ...
    ) {
        ...
        this.session = bghttpModule.session('image-upload-bg-http');
    }

    private sendImages(fileUri): Q.Promise<any> {
        const defer = Q.defer();

        const imageName = this.extractImageName(fileUri);

        const request = {
            url: ... [[ some rest api url ]],
            method: 'PATCH',
            headers: {
                'Content-Type': 'application/octet-stream',
                'File-Name': imageName,
                'Content-Disposition': `attachment; filename=${imageName}`,
                Authorization: ... [[anonymized]],
            },
            description: "{ 'uploading': " + imageName + ' }',
        };

        const task = this.session.uploadFile(fileUri, request);

        task.on('progress', logEvent);
        task.on('error', err => {
            defer.reject(err);
        });
        task.on('complete', () => {
            defer.resolve();
        });

        function logEvent(e) {
            console.log('currentBytes: ' + e.currentBytes);
            console.log('totalBytes: ' + e.totalBytes);
            console.log('eventName: ' + e.eventName);
        }

        return defer.promise;
    }

Here is the error log I have.

JS: === dump(): dumping members ===
JS: {
JS:     "eventName": "error",
JS:     "object": {
JS:         "_observers": {
JS:             "progress": [
JS:                 {
JS:                     "callback": "callback()function logEvent(e) {\n            console.log('currentBytes: ' + e.currentBytes);\n            console.log('totalBytes: ' + e.totalBytes);\n            console.log('eventName: ' + e.eventName);\n        }"
JS:                 }
JS:             ],
JS:             "error": [
JS:                 {
JS:                     "callback": "callback()function (err) {\n            defer.reject(err);\n        }"
JS:                 }
JS:             ],
JS:             "complete": [
JS:                 {
JS:                     "callback": "callback()function () {\n            defer.resolve();\n        }"
JS:                 }
JS:             ]
JS:         },
JS:         "_session": {
JS:             "_id": "image-upload-bg-http"
JS:         },
JS:         "_id": "image-upload-bg-http{1}",
JS:         "_description": "{ 'uploading': random-avatar-2.png }",
JS:         "_upload": 0,
JS:         "_totalUpload": 1,
JS:         "_status": "error"
JS:     },
JS:     "error": null
JS: }
JS: === dump(): dumping function and properties names ===
JS: === dump(): finished ===
fthuin commented 6 years ago

Related to #117, but the name of the issue is not correct, it's the name of the channel of notification that is null and not the namespace.

It looks like it is only happening with nativescript-angular.

fthuin commented 6 years ago

It looks like for Android 8.0 we have to create a NotificationChannel.

The code to do it in Java is located at https://github.com/gotev/android-upload-service/blob/master/uploadservice/src/main/java/net/gotev/uploadservice/UploadTask.java in the function init()

    protected void init(UploadService service, Intent intent) throws IOException {
        this.notificationManager = (NotificationManager) service.getSystemService(Context.NOTIFICATION_SERVICE);
        this.params = intent.getParcelableExtra(UploadService.PARAM_TASK_PARAMETERS);
        this.service = service;
        this.mainThreadHandler = new Handler(service.getMainLooper());

        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && params.notificationConfig != null) {
            String notificationChannelId = params.notificationConfig.getNotificationChannelId();

            if (notificationChannelId == null) {
                params.notificationConfig.setNotificationChannelId(UploadService.NAMESPACE);
                notificationChannelId = UploadService.NAMESPACE;
            }

            if (notificationManager.getNotificationChannel(notificationChannelId) == null) {
                NotificationChannel channel = new NotificationChannel(notificationChannelId, "Upload Service channel", NotificationManager.IMPORTANCE_LOW);
                notificationManager.createNotificationChannel(channel);
            }
        }

    }

But it looks like this part is never called.

A quick fix is to modify in background-http-android.ts

        const displayNotificationProgress = typeof options.androidDisplayNotificationProgress === "boolean" ? options.androidDisplayNotificationProgress : true;
        if (displayNotificationProgress) {
            const channel = new android.app.NotificationChannel(application.android.packageName, application.android.packageName,
                android.app.NotificationManager.IMPORTANCE_LOW);
            const notificationManager = context.getSystemService(android.content.Context.NOTIFICATION_SERVICE)
            notificationManager.createNotificationChannel(channel);

            const uploadNotificationConfig = new net.gotev.uploadservice.UploadNotificationConfig();
            uploadNotificationConfig.setNotificationChannelId(application.android.packageName)
            request.setNotificationConfig(uploadNotificationConfig);
        }

There is a missing check for the API version but I don't know if a real fix in a pull request would not be better.

NickIliev commented 6 years ago

@fthuin iPR fixing the issue in the source code would be great! We are welcoming all contributors so if you have the time feel free to join the community by creating your PR!

fthuin commented 6 years ago

@NickIliev It's here #126 😄

LorenDorez commented 6 years ago

@NickIliev DO we know when this PR will get accepted? I have an app that i upgraded and its dead with 3.2.2. I would rather see the update than roll back to 3.2.0

I have tested the PR in my project and it appears to be working

DimitarTodorov commented 6 years ago

The issue is fixed in version 3.2.3, which is already released.

FranciZ commented 6 years ago

@DimitarTachev Just tested 3.2.3 and it's not fixed.

The version is 3.2.3. The error I get is the same as with 3.2.2.

{"eventName":"error","object":{"_observers":{"progress":[{}],"error":[{}],"complete":[{}]},"_session":{"_id":"image-upload"},"_id":"image-upload{1}","_description":"{ 'uploading': NSIMG_20180415_211128.jpg }","_upload":0,"_totalUpload":1,"_status":"error"}}

Altering the code as per @fthuin's comment gets it working. But as per my testing the 3.2.3 is not working on 8.0.0.

Please reopen.

DimitarTachev commented 6 years ago

@FranciZ, we are not able to reproduce any errors with the latest version. I suggest you test it with some of the Demo apps of the plugin and if you are still getting an exception, please open a new issue with exact steps to reproduce or a sample application.

FranciZ commented 6 years ago

@DimitarTachev, interesting. As I said, the fix mentioned here fixes the issue. Without this, using the version 3.2.3 the error is the same as mentioned in the original issue without this fix applied.

DimitarTachev commented 6 years ago

@FranciZ, the reason I closed this one was that the issues of @fthuin and @LorenDorez were resolved and their apps are working as expected with the latest version of the plugin. I already had a discussion with them in the Pull Requests and the community slack.

Without providing us with a sample application or any steps to reproduce the error in one of the Demo apps, I can only suggest you try removing your platforms folder and rebuilding your app as everything is working as expected on our side.

fthuin commented 6 years ago

I confirm that everything is working well on my side. Do not hesitate to force the last version by updating your package.json, removing node_modules and platforms and do a clean run.

FranciZ commented 6 years ago

@fthuin Thanks for the feedback! Must be something wrong on my end.

luis-abeno commented 5 years ago

Tks, saved my day.