Hi,
We started using your code-push-server
and code-push-cli
with capacitor plugin https://github.com/mapiacompany/capacitor-codepush and we found some problems. I believe that these problems are not present when used with the react-native release/plugins.
Most serious one is that second release will fail all the time if the output folder is not named CodePush
. So "partial" release functionality is broken.
Steps to reproduce:
- prepare code push:
code-push app add test-android android cordova
- create new folder and file with some content:
echo "version 1" > ./test_folder/test_file
- create release:
code-push release test-android ./test_folder "*"
- change file:
echo "version 2" > ./test_folder/test_file
- release it again:
code-push release test-android ./test_folder "*"
there is error on the server, and server instance will stop! (we are not using pm2, just simple node process)
Error: ENOENT: no such file or directory, stat 'C:\....\code-push-demo\workDir\codepush_Ye3hArNTWWsMkBtBAsGeLMEV04BID9dM\e592ede40edcc1a367b8c19d67de83811848f9345b682c39a2efff199a2f8a37\dataCenter\CodePush\test_file'
Emitted 'error' event on ZipFile instance at:
at C:\...\code-push-demo\node_modules\yazl\index.js:31:26
I tried to investigate and found two problems:
First, this error should not kill whole node process. This is caused by improper error handling on zip file creation: on https://github.com/shm-open/code-push-server/blob/master/src/core/services/package-manager.ts#L233 it is not sufficient to add error handler to zipFile.outputStream.on('error'...)
.
If we check how ZipFile is throwing exeption https://github.com/thejoshwolfe/yazl/blob/master/index.js
we can see it used like: if (err) return self.emit("error", err);
and util.inherits(ZipFile, EventEmitter);
. So to catch these exeptions we need to add error handler on the zipFile instance: zipFile.on('error', (error)=>{ reject(error);})
.
After this, the error will not kill whole server. Also after restart of the server everything looks ok-ish, because even if partial release failed to be created, the full release is used as fallback. So if someone is using for example pm2
this error could be unnoticed.
Second problem is error itself. The problem is that on https://github.com/shm-open/code-push-server/blob/master/src/core/utils/security.ts#L186-L189, when you are creating manifest file, the root folder is renamed to CodePush
. So in my example of test_folder/test_file
, the created manifest would contain something like this:
{
'CodePush/test_file': 'b03d44cd60d71de68a4aca7808c6f768802f6d6c414430ff8ccea10c1aa57b4c'
}
Then on the https://github.com/shm-open/code-push-server/blob/master/src/core/services/package-manager.ts#L287 in the generateOneDiffPackage
function there is comparison of old and new manifest files. The result in my example run is ['CodePush/test_file']
. Then this path is used to create zip file containing only changed files. On line https://github.com/shm-open/code-push-server/blob/master/src/core/services/package-manager.ts#L241 there is zipFile.addFile(path.join(baseDirectoryPath, file), slash(file));
but this file does not exist. In my example path is C:\....\code-push-demo\workDir\codepush_Ye3hArNTWWsMkBtBAsGeLMEV04BID9dM\e592ede40edcc1a367b8c19d67de83811848f9345b682c39a2efff199a2f8a37\dataCenter\CodePush\test_file
. But without renaming it would be: C:\....\code-push-demo\workDir\codepush_Ye3hArNTWWsMkBtBAsGeLMEV04BID9dM\e592ede40edcc1a367b8c19d67de83811848f9345b682c39a2efff199a2f8a37\dataCenter\test_folder\test_file
which exists.
I think if works for you only because during code-push release-react
you are writing files to the folder named CodePush
. From cli console: node node_modules\react-native\local-cli\cli.js bundle --assets-dest C:\***\Temp\CodePush\CodePush --bundle-output C:\***\Temp\CodePush\CodePush\index.android.bundle .....
But also react native apps do not need the renaming, in that case the renaming is: CodePush
->CodePush
(if code-push release-react
is used or bundle/asset outpus is in CodePush
folder). I think the correct fix should be to remove lines https://github.com/shm-open/code-push-server/blob/master/src/core/utils/security.ts#L186-L189.
In real word scenario the cordova builds have builded files in www
folder and ionic capacitor .../public
folder. In both cases the rename of the folder to CodePush
in the manifest paths will result in error on second release.
I found this issue mentioned few times:
#75 (comment) They "fixed" issue by replacing rename.
the same thing here: lisong#319
The rename code originate from this PR: https://github-com.translate.goog/lisong/code-push-server/pull/236?_x_tr_sl=auto&_x_tr_tl=en&_x_tr_hl=en&_x_tr_pto=wapp
Where they created react native build, but not in CodePush
folder. I think the proper solution was either to change output folder, or fix hardcoded CodePush
folder in the ios plugin. But still I do not understand how it was possible that they did not get the same error that we have now.
Just to double check if the partial update feature was not destroyed after I removed rename code. I tried it and it works without problems on our capacitor app: (note only one src_app_pages_profile_profile_module_ts file was changed)
VM3:192 result Filesystem.readdir (#100067870)
VM3:203 Objectfiles: (2) ['src_app_pages_profile_profile_module_ts.js', 'src_app_pages_profile_profile_module_ts.js.map'][[Prototype]]: Object
VM3:218 native Filesystem.stat (#100067871)
VM3:225 ObjectcallbackId: "100067871"methodName: "stat"options: {directory: 'DATA', path: 'codepush/download/unzipped/public/src_app_pages_profile_profile_module_ts.js'}pluginId: "Filesystem"[[Prototype]]: Object
VM3:192 result Filesystem.stat (#100067871)
....
VM3:192 result Filesystem.readFile (#100067875)
src_app_pages_profile_profile_module_ts.js:1088 [CodePush] Applying diff update
Thanks!
(I can create pull request if you accept, but I don't know how to test react-native/ios part)