Skip to content

Cannot read property 'split' of undefined #538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pelargir opened this issue Dec 18, 2019 · 19 comments · Fixed by #681
Closed

Cannot read property 'split' of undefined #538

pelargir opened this issue Dec 18, 2019 · 19 comments · Fixed by #681
Assignees
Labels

Comments

@pelargir
Copy link

pelargir commented Dec 18, 2019

2.1.0 is tagged as pre-release so I assume it's not production-ready yet. We attempted to upgrade and immediately began getting these JS errors which are not present in the prior version:

71085238-11119580-2165-11ea-94b9-b8e23a1675ff

71085440-8c734700-2165-11ea-8dcf-44ff46c36bef

@tresf
Copy link
Contributor

tresf commented Dec 18, 2019

Can you please provide the steps necessary to reproduce these errors?

  • We'll need exact versions of QZ Tray and qz-tray.js (or we'll assume 2.1.0)
  • We'll need code snippets to reproduce the problem

@pelargir
Copy link
Author

QZ Tray v2.0.11-1
qz-tray.js 2.1.0

I haven't isolated the code that's causing the problem yet, but will post when I do.

@tresf
Copy link
Contributor

tresf commented Dec 20, 2019

@pelargir thanks. It's probably an overlooked entry in our compatible: function. We'd be happy to patch it once it's identified.

@tresf
Copy link
Contributor

tresf commented Dec 29, 2019

I haven't isolated the code that's causing the problem yet, but will post when I do.

@pelargir checking in to see if you can provide some sample code, we'd love to fix this. If we don't see the code in another week or so, we'll close this out, but we'd happily re-open it when the code is provided.

@pelargir
Copy link
Author

Closing for now. I definitely won't be able to provide anything within a week.

@pelargir
Copy link
Author

I've tracked the problem down to qz-tray.js on line 650:

compatible: function(printData) {
  var semver = _qz.websocket.connection.version.split(/[.-]/g);
  ...

The split method is being called on _qz.websocket.connection.version which is null. Here's a stack trace from the browser console:

qz-tray.js:705 Uncaught (in promise) TypeError: Cannot read property 'split' of undefined
    at Object.compatible (qz-tray.js:705)
    at Object.print (qz-tray.js:1283)
    at sendPrintJob (cozy-printer.js:68)

Any idea why the version would be null?

@pelargir pelargir reopened this Mar 22, 2020
@tresf
Copy link
Contributor

tresf commented Mar 22, 2020

That should only be null if you're not connected (the value is filled on successful connection).

@tresf
Copy link
Contributor

tresf commented Mar 22, 2020

We did see this error for some customers and it was caused by a behavior change in the WebSocket as described here: #524

That fix was Java related, as the WebSocket would throw an unrecoverable error with certain browsers (Mostly Microsoft Edge), but that should be fixed with 2.1.0 proper.

@tresf
Copy link
Contributor

tresf commented Apr 20, 2020

Closing as fixed for now, since we can't reproduce and all previous errors were fixed in code. Please request a re-open if steps to reproduce can be provided.

@pelargir
Copy link
Author

This is no longer happening when using qz-tray.js v2.1.1 with QZ Tray 2.0.12 client.

@tresf
Copy link
Contributor

tresf commented Jun 18, 2020

@pelargir thanks for the follow up, much appreciated.

@pelargir
Copy link
Author

pelargir commented Jun 22, 2020

Spoke too soon. This is actually still happening, albeit with a slightly different error message. The problem still seems to center around receiving an unexpected null QZ Tray semver.

I'll open a separate issue with additional information since the error is different.

@tresf
Copy link
Contributor

tresf commented Jun 22, 2020

Although the error has technically changed, I expect the underlying cause to be identical. We're always fine with fresh bug reports, it helps others/search engines find the problem quicker. :D

@pelargir
Copy link
Author

Thanks for your patience. I won't be opening a new issue after all. I fixed the new error by updating our print options from 2.0 to the 2.1 syntax. I also updated QZ Tray client to 2.1. I had thought qz-tray.js 2.1 could still operate nicely with QZ Tray client 2.0, but in our situation that didn't seem to be happening. It's working now though. 👍

@tresf
Copy link
Contributor

tresf commented Jun 23, 2020

I had thought qz-tray.js 2.1 could still operate nicely with QZ Tray client 2.0

getVersion has been in the API since 2.0 and 2.1 is 100% backwards compatible with 2.0. Can you provide a code snippet and exact version that this happens on? We're committed to fixing backwards compatibility. QZ Tray 2.0 is supported until the end of the year and we still have a lot of clients running it.

@pelargir
Copy link
Author

pelargir commented Jun 25, 2020

Sure, I'll see what I can put together for you regarding the version incompatibility we observed.

Incidentally, after deploying our most recent v2.1 syntax changes to our production environment, the "undefined" version error came back again. We finally switched to establishing our websocket connection on page load (instead of immediately before attempting to print as it had been before) and the problem went away.

It seems like the websocket connection was not being opened quickly enough and as a result, when we tried to print, the QZ Tray version was coming back undefined. Subsequent attempts to print (without reloading the page) worked since the connection had certainly been established by that point.

This was the code we had been using when we saw the "undefined" version errors:

qz.websocket.connect().then(sendPrintJob).catch(reject)

This was what ended up fixing the problem:

qz.websocket.connect()

// other stuff in between

sendPrintJob()

When a websocket connection is first established, is there a delay during which API calls would fail (i.e. the QZ Tray version cannot be successfully retrieved)? If so, I would have thought the promise chain above would not have allowed sendPrintJob to execute before the connection was fully established.

@tresf
Copy link
Contributor

tresf commented Jun 25, 2020

qz.websocket.connect().then(sendPrintJob).catch(reject)

Correct, I'd expect this to work. Testing locally, this is fine:

qz.websocket.connect().then(
   ()=> qz.printers.getDefault()
).then(
   (printer)=> qz.print(qz.configs.create(printer), [ { type: 'pixel', format: 'html', flavor: 'file', data: 'https://qz.io/about' } ])
).catch(
   (err) => console.error(err)
);

Note, there's some implicit stuff happening above, getDefault() returns a promise, print() returns a promise, JavaScript returns these objects since they're the last line of the function. They can be explicitly returned using .then(function() { return qz.printers.getDefault(); }). Perhaps we can do a screenshare (email [email protected]) so we can take a look firsthand. I don't want others to suffer an internal race condition upon upgrade, that'd be terrible for developers and users.

@tresf tresf reopened this Jun 26, 2020
@tresf
Copy link
Contributor

tresf commented Jun 26, 2020

As most race conditions are -- this one was hard to reproduce. Further examining the code, I believe the connection should wait until getVersion() has completed first.

@pelargir
Copy link
Author

pelargir commented Jun 26, 2020

Here's our code:

if (qz.websocket.isActive()) {
  qz.print(config, data).then(() => {
    resolve()
  }).catch(reject)
} else {
  qz.websocket.connect().then(() => {
    qz.print(config, data)
  }).then(() => {
    resolve()
  }).catch(reject)
}

@tresf tresf changed the title JS errors in 2.1.0 pre-release version Cannot read property 'split' of undefined Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants