search

FGasper / zmodemjs

zmodem.js - ZMODEM in JavaScript
Apache License 2.0
128 stars 25 forks source link

After upgrade from 0.1.7 to 0.1.9 rz do not work any more #5

Closed zxdong262 closed 5 years ago

zxdong262 commented 5 years ago

I will compare the code later to see what happens.

zxdong262 commented 5 years ago

Seems the main change is here: https://github.com/FGasper/zmodemjs/commit/edd14ccbbd8fb384482e712db23e33fc9fea5033

But I do not understand the code.

FGasper commented 5 years ago

Can you please describe the failure you are seeing?

Do the tests pass on your machine? They do flex interaction with rz.

-F

On Jun 12, 2019, at 10:22 PM, ZHAO Xudong notifications@github.com wrote:

I will compare the code later to see what happens.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

tsl0922 commented 5 years ago

Same here, rz is unusable on ttyd now, but sz works fine.

[ttyd] zmodem consume:  Error: Unhandled header: ZRPOS
    at ZmodemSendSession._consume_header (zsession.js:308)
    at ZmodemSendSession._parse_and_consume_header (zsession.js:294)
    at ZmodemSendSession._consume_first (zsession.js:1599)
    at ZmodemSendSession.consume (zsession.js:229)
    at ZmodemSentry.consume (zsentry.js:228)
    at Xterm.onSocketData (index.tsx:283)
FGasper commented 5 years ago

I do see a test failure now … odd that the CI didn’t catch it.

FGasper commented 5 years ago

Test failure is unrelated.

tsl0922 commented 5 years ago

@FGasper Is there anything that I can help?

You can reproduce the error above on ttyd:

  1. download a ttyd binary from: https://github.com/tsl0922/ttyd/releases/tag/1.5.0
  2. run ./ttyd_linux.x86_64 bash
  3. open http://localhost:7681
  4. type rzin the web terminal.

If you want to run a development version of the frontend, follow the README here.

FGasper commented 5 years ago

Looking.

FGasper commented 5 years ago

@tsl0922 Are you able to make a build that uses this branch: https://github.com/FGasper/zmodemjs/tree/debugging_on

I’m not sure why it’s complaining about an unrecognized header. How can I run ttyd but be able to edit the JS code that it’s running?

tsl0922 commented 5 years ago

You can run the dev server: https://github.com/tsl0922/ttyd/tree/master/html

FGasper commented 5 years ago

Those instructions don’t work:

felipe@felipes-mbp 22:15:25 ~/code/ttyd/html
> yarn install
yarn install v1.12.1
[1/4] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.33s.

OK
felipe@felipes-mbp 22:15:48 ~/code/ttyd/html
> ttyd bash
-bash: ttyd: command not found
FGasper commented 5 years ago

Also:

> yarn run start
yarn run v1.12.1
error Command "start" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
FGasper commented 5 years ago

@zxdong262 How can I reproduce the issue as you’ve seen it?

zxdong262 commented 5 years ago

Sorry for the delay.

You can start dev mode:(mac or linux)

git clone git@github.com:electerm/electerm.git
cd electerm
# then edit package.json, set zmodem.js version to 0.1.9 to reproduce the bug

# then install deps
npm i

# in separate terminal, run webpack dev server
npm run c

# in separate terminal, start electerm dev(electerm app, you can view console by open developer tool from menu left top corner)
npm run app

# make sure you connect to a ssh by click "+" icon, not from local terminal

# then you can edit zmodem in node_modules/zmodem.js/*** to trigger rebuild and auto refresh
tsl0922 commented 5 years ago

@FGasper yarn run start should bu run in the html dir.

You can get a ttyd binary from: https://github.com/tsl0922/ttyd/releases/tag/1.5.0, or build it yourself.

FGasper commented 5 years ago

@FGasper yarn run start should bu run in the html dir.

That’s what I’m doing.

FGasper commented 5 years ago

@zxdong262 It works if I say npm run compile rather than npm run c.

But I don’t see anything of use when I run the application. I do see this in the terminal where I run it:

platform: darwin
00:51:19 › error parsing $HOME/.ssh/config
00:51:19 › maybe no $HOME/.ssh/config, but it is ok
00:51:20 › no ../../install-src file
00:51:20 › no install-src file
00:51:20 › App starting...
00:51:20 › no /Users/felipe/Library/Application Support/electerm-config.js but it is ok
00:51:20 › no ../../install-src file
00:51:20 › no install-src file
00:51:20 › server runs on localhost 3075
00:54:32 › close app
tsl0922 commented 5 years ago

@FGasper I think there may be something wrong with your local environment, this is what I get:

image

FGasper commented 5 years ago

@tsl0922 You haven’t defined the “start” script in your package.json file; I assume that’s why it’s not working here. Maybe you have some special setup on your end?

For example, I can run yarn run build, and it finishes as expected. But that’s the only script you’ve defined.

FGasper commented 5 years ago

I’ve added another demonstration of how the send session logic works: bin/zmodemjs-sz.js in the master branch.

Still trying to get reproduction on the problem. @tsl0922 npm run start works on Linux but not macOS … curious.

zxdong262 commented 5 years ago

@FGasper Some node module need build with node-gyp, might need python 2.x or python 3.6+, I guess, you may try npm i -g node-gyp first, and rm -rf node_mdules && npm i.

https://github.com/nodejs/node-gyp, seems no 3.x support, need python 2.x

tsl0922 commented 5 years ago

@FGasper The screenshot above is already running on macOS... I can confirm there is a "start" script on the latest master branch.

https://github.com/tsl0922/ttyd/blob/master/html/package.json#L14

FGasper commented 5 years ago

@tsl0922

image

^^ rz works for me on Linux.

I apparently had an outdated repo in macOS. Going to retry now.

tsl0922 commented 5 years ago

@FGasper Yes, but rz only works for small files on linux, and it always fail on macOS's brew installed rz, strange.

FGasper commented 5 years ago

@tsl0922 I’m seeing logs about SIGHUP and “Connection closed” in the browser when I send a 10-MiB file … is that what you mean?

[2019/06/24 01:23:52:1498] NOTICE: sending SIGHUP (1) to process (group) -9393
[2019/06/24 01:23:52:1505] NOTICE: WS closed from 172.16.92.1 (172.16.92.1), clients: 0
[2019/06/24 01:23:52:1581] NOTICE: process exited with code 1, pid: 9393
[2019/06/24 01:24:04:3583] NOTICE: WS   /ws - 172.16.92.1 (172.16.92.1), clients: 1
[2019/06/24 01:24:04:5341] NOTICE: started process, pid: 9484
FGasper commented 5 years ago 
[2019/06/24 01:25:45:7224] ERR: write INPUT to pty: 35 (Resource temporarily unavailable)
[2019/06/24 01:25:49:5866] NOTICE: sending SIGHUP (1) to process (group) -7484
[2019/06/24 01:25:49:5875] NOTICE: WS closed from 127.0.0.1 (localhost), clients: 0
[2019/06/24 01:25:49:6259] NOTICE: process exited with code 1, pid: 7484
[2019/06/24 01:25:50:1881] NOTICE: WS   /ws - 127.0.0.1 (localhost), clients: 1
[2019/06/24 01:25:50:1904] NOTICE: started process, pid: 7527

^^ I also see the above when I try to send the file in macOS.

tsl0922 commented 5 years ago

Yes, you can view the error log in the browser console. I don't know how to cancel the zmodem session when error occurs, so the websocket connection is closed/reopened to force the server side sz/rz to be killed.

FGasper commented 5 years ago

How do I get the webpack server to rebuild zmodem.js if I make changes?

It seems like that’s what the webpack dev server is supposed to do, but it’s not doing it.

tsl0922 commented 5 years ago

It should rebuild automatically (on macOS) when file changes, or you can re run the start command.

FGasper commented 5 years ago

It should rebuild automatically (on macOS) when file changes, or you can re run the start command.

This doesn’t seem to be working. I’m making changes to ttyd/html/node_modules/zmodem.js/src/zsession.js, but I’m not seeing the changes in the browser, even after I rerun npm run start.

I’ve just added another test to the master branch that flexes the rz logic against a large file.

tsl0922 commented 5 years ago

Well, it only works for the src dir, node_modules dir is a monster, it's not watched by default 😂.

FGasper commented 5 years ago

Well that’s not helpful. :-P Anyway, I’ve got it spitting diag statements after publishing. Hang on.

FGasper commented 5 years ago

OK.

In a normal single-file session the flow is:

That antepenultimate ZRINIT is what’s not happening; your rz process is instead sending another ZRPOS. I think that means rz isn’t receiving all of the data that it should.

FGasper commented 5 years ago

I’ve added a test to master for sending a 1-MiB file of all random octets.

At this point I’m not sure what to tell you. rz says that it’s not getting everything that it expects. My own (several) implementations have no such problems.

I did also try rz -vvvvvvvvv 2>rz_diag; here is that output file:

rz 0.12.20

mode:1
rz waiting to receive.zshhdr: ZRINIT 23000000Calling read: alarm=10  Readnum=8192 Read returned 27 bytes
zgethdr: ZSINIT 40000000zrdata: 1  ZCRCWzshhdr: ZACK 1Calling read: alarm=10  Readnum=8192 Read returned 68 bytes
zgethdr: ZFILE 0zrdata: 46  ZCRCWmode:3
zmanag=0, Lzmanag=0
zconv=1

Receiving: 02-Sicut.mp3
zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 15 bytes
zgethdr: ZDATA 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Data subpacket too long
zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Got ERROR
zgethdr: ERROR 0zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 27843e0bzshhdr: ZRPOS 0Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR c0e29e8fzshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Garbage count exceeded
zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 35ffe442zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR a62086d9zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 9246a886zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR fc4b468zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR b52f3934zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Got ERROR
zgethdr: ERROR b52f3934zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Got ERROR
zgethdr: ERROR b52f3934zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 74e9c44ezshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Got ERROR
zgethdr: ERROR 74e9c44ezshhdr: ZRPOS 0Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Retry 0: Got ERROR
zgethdr: ERROR 74e9c44ezshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 1935c3d7zshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Got ERROR
zgethdr: ERROR 1935c3d7zshhdr: ZRPOS 0Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 7015bfd1zshhdr: ZRPOS 0Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=1  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR edb44bcbzshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 883d4d5azshhdr: ZRPOS 0Calling read: alarm=10  Readnum=8192 Read returned 1022 bytes
Retry 0: Bad CRC
Retry 0: Got ERROR
zgethdr: ERROR 3588d893rzfile: zgethdr returned -1

rz: 02-Sicut.mp3 removed.
mode:0

Transfer incomplete
FGasper commented 5 years ago

@zxdong262 Is there an easy way to update master to use zmodem.js 0.1.9? I see that it uses 0.1.7 currently; when I tried updating package.json and “npm i” it seemed to have no effect.

zxdong262 commented 5 years ago

@FGasper It should install 0.1.9 after npm i, maybe your npm is too old? You can try rm -rf node_modules/zmodem.js && npm i

FGasper commented 5 years ago

@zxdong262 OK, I have 0.1.9 running now, but rz works for me.

There are three changes from the commit you identified: 2 that are just order of promise resolution, and 1 that makes a promise resolve to undefined rather than to another promise. If you’re still having a problem, do you mind reverting each of those individually and seeing if you can identify a particular problem?

Note that the xterm.js plugin is basically unmaintained at this point. I have no use for it anymore, and xterm.js’s plugin API has changed completely from when I used it. It’s either slated for removal from their repository or is already removed.

FGasper commented 5 years ago

It would be most helpful if you’d be able to write a test that demonstrates the breakage you’re seeing.

zxdong262 commented 5 years ago

@FGasper Thank you, I would test again as you described.

zxdong262 commented 5 years ago

@FGasper When I import from src, it works, but load zmodem.js directly will not. 

# this will work
import Zmodem from 'zmodem.js/src/zmodem_browser'

I noticed zmodem.js/index.js requires src/zsentry, is it the problem? or the build process? Can you check it.

FGasper commented 5 years ago

The build process hasn’t changed from 2 years ago when I integrated zmodem.js with xterm.js. If there’s a problem with it now, it’s likely breakage upstream or improper use.

Do you see a problem with the build?

On Jun 25, 2019, at 4:46 AM, ZHAO Xudong notifications@github.com wrote:

@FGasper When I import from src, it works, but load zmodem.js directly will not.

this will work

import Zmodem from 'zmodem.js/src/zmodem_browser' I noticed zmodem.js/index.js requires src/zsentry, is it the problem? or the build process? Can you check it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tsl0922 commented 5 years ago

@FGasper I'm trying to reproduce this issue with the new bin/zmodemjs-sz.js, but still getting errors, can you fix it? This is my test script (works well with the compiled lrz/lsz):

#!/bin/bash

set -eo pipefail
set -x

file=$1

# sz="/Users/tsl0922/lrzsz-0.12.20/src/lsz"
sz="node /Users/tsl0922/zmodemjs/bin/zmodemjs-sz.js"
rz="/Users/tsl0922/lrzsz-0.12.20/src/lrz"

testdir="test.rzsz"
rm -rf $testdir
mkdir -p $testdir
mkfifo $testdir/pipe || mknod $testdir/pipe p

$sz $file < $testdir/pipe | (cd $testdir; exec $rz >> ./pipe)

log:

$ ./test.sh Downloads/googlechrome.dmg
+ file=Downloads/googlechrome.dmg
+ sz='node /Users/tsl0922/zmodemjs/bin/zmodemjs-sz.js'
+ rz=/Users/tsl0922/lrzsz-0.12.20/src/lrz
+ testdir=test.rzsz
+ rm -rf test.rzsz
+ mkdir -p test.rzsz
+ mkfifo test.rzsz/pipe
+ node /Users/tsl0922/zmodemjs/bin/zmodemjs-sz.js Downloads/googlechrome.dmg
+ cd test.rzsz
+ exec /Users/tsl0922/lrzsz-0.12.20/src/lrz
lrz waiting to receive./Users/tsl0922/zmodemjs/src/zsession.js:309
        var handler = this._next_header_handler[ new_header.NAME ];
                                               ^

TypeError: Cannot read property 'ZRINIT' of undefined
    at ZmodemSendSession._consume_header (/Users/tsl0922/zmodemjs/src/zsession.js:309:48)
    at ZmodemSendSession._parse_and_consume_header (/Users/tsl0922/zmodemjs/src/zsession.js:298:14)
    at ZmodemSendSession._consume_first (/Users/tsl0922/zmodemjs/src/zsession.js:1609:19)
    at ZmodemSendSession.consume (/Users/tsl0922/zmodemjs/src/zsession.js:231:18)
    at ReadStream.stdin.on (/Users/tsl0922/zmodemjs/bin/zmodemjs-sz.js:128:22)
    at ReadStream.emit (events.js:198:13)
    at addChunk (_stream_readable.js:288:12)
    at readableAddChunk (_stream_readable.js:269:11)
    at ReadStream.Readable.push (_stream_readable.js:224:10)
    at lazyFs.read (internal/fs/streams.js:181:12)
Retry 0: Got TIMEOUT
lrz: caught signal 13; exiting
FGasper commented 5 years ago

@tsl0922 I’ve fixed the script, but the issue was there, not in the actual zmodem code.

I’m going to close this issue tomorrow unless there is more information given about the breakage.

tsl0922 commented 5 years ago

I enabled the new DEBUG flag on master to watch if it matches the "single-file session flow" you given above, then I found the issue: sz (zmodem.js) never sends a ZFIN (should be sent via the session.close() call), so I wrote a setInterval to do this this (a hack), and it works now (for small files, eg: data less than 1000 bytes).

Zmodem.Browser.send_files never resolves on ttyd's master branch too, do you think this is a issue?

You can reproduce it using this branch of ttyd: https://github.com/tsl0922/ttyd/tree/zmodem

FGasper commented 5 years ago

@tsl0922 The script you gave demonstrates this working correctly:

> bash rzsz_pipe.sh single
+ file=single
+ sz='node /Users/felipe/code/zmodemjs/bin/zmodemjs-sz.js'
+ rz=rz
+ testdir=test.rzsz
+ rm -rf test.rzsz
+ mkdir -p test.rzsz
+ mkfifo test.rzsz/pipe
+ node /Users/felipe/code/zmodemjs/bin/zmodemjs-sz.js single
+ cd test.rzsz
+ exec rz
rz waiting to receive.send SENDING HEADER ZSINIT_HEADER { _bytes4: [ 0, 0, 0, 64 ] }
send RECEIVED HEADER ZACK_HEADER { _bytes4: [ 1, 0, 0, 0 ] }
send SENDING HEADER ZFILE_HEADER { _bytes4: [ 0, 0, 0, 0 ] }
Receiving: single
send RECEIVED HEADER ZRPOS_HEADER { _bytes4: [ 0, 0, 0, 0 ] }
send SENDING HEADER ZDATA_HEADER { _bytes4: [ 0, 0, 0, 0 ] }
Bytes received:      44/     44   BPS:15839  ETA 00:00  send SENDING HEADER ZEOF_HEADER { _bytes4: [ 44, 0, 0, 0 ] }
Bytes received:      44/     44   BPS:8565
send RECEIVED HEADER ZRINIT_HEADER { _bytes4: [ 0, 0, 0, 99 ] }
send SENDING HEADER ZFIN_HEADER { _bytes4: [ 0, 0, 0, 0 ] }
send RECEIVED HEADER ZFIN_HEADER { _bytes4: [ 0, 0, 0, 0 ] }
FGasper commented 5 years ago

@tsl0922 Every automated test I have also demonstrates the ZmodemSendSession object sending a ZFIN.

I suspect the issue here is in your implementation. You might try console.warn() in your send function.

Last call: can either of you demonstrate a reproducible error that was introduced from 0.1.7 to 0.1.9?

tsl0922 commented 5 years ago

No, maybe you can provide a browser demo with node-pty on your repo to help us, thanks.

rz never works good on ttyd, so this issue has been going on for a long time. I spent a lot of time trying to debug and resolve it, but got no luck.

I'm considering remove zmodem support on ttyd for the bad experience.

FGasper commented 5 years ago

@tsl0922 If this is a longstanding problem, then can you please open a new issue that describes the specific issue you’re seeing?

This problem as reported is that a regression happened from 0.1.7 to 0.1.9; based on what you’re saying now, that doesn’t, in fact, appear to be what you’re seeing.

If the issue is that zsession.js isn’t sending a ZFIN, you should be able to diagnose that relatively simply. Given that it works fine over simple IPC, it seems more likely that whatever transport you’re using may not be sending that ZFIN. Again, that should be relatively easy to diagnose.

If the problem only happens when node-pty is involved, then node-pty may be the source of your issue.

tsl0922 commented 5 years ago

OK, you can close this issue. I will port the C version to the browser with emscripten, and compare it with zmodem.js.

tsl0922 commented 5 years ago

@FGasper I've figured out it. You are right 👍, it's not zmodemjs's issue.

Both ttyd and node-pty uses none-block IO now, which breaks rz (should be the reason why the zmodem demo in xterm.js repo don't work anymore). After removing the O_NONBLOCK flag from the pty fd, everything works fine now on ttyd.

FGasper commented 5 years ago

Non-blocking I/O in node-pty shouldn’t matter to rz; either side of a pipe/tty/socket can be blocking or non-blocking independently of the other.

That said, I’m glad you’ve found a configuration that works for you. :)