Skip to content
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

Error handling when setSignaturePromise() fails #1273

Closed
ThomasRutzer opened this issue Jul 9, 2024 · 8 comments · Fixed by #1274
Closed

Error handling when setSignaturePromise() fails #1273

ThomasRutzer opened this issue Jul 9, 2024 · 8 comments · Fixed by #1274

Comments

@ThomasRutzer
Copy link

ThomasRutzer commented Jul 9, 2024

Similar to issue #835, it is also difficult to recognise from the client's perspective when a signature request fails. It appears that data is still being sent via the web socket connection here without a traceable error/warning that could be handled in the client.

In our case, we would like to stop the printing process if the signature fails. Does anyone have any tips on how we can achieve this?

@tresf
Copy link
Contributor

tresf commented Jul 9, 2024

Can you create a minimal example?

@ThomasRutzer
Copy link
Author

ThomasRutzer commented Jul 9, 2024

Is there any boilerplate / CodePen with a setup to start from?

@tresf
Copy link
Contributor

tresf commented Jul 9, 2024

Is there any boilerplate / CodePen with a setup to start from?

I sort of asked you for this, but for sake of moving forward with this bug report:
https://jsfiddle.net/mjcqt95p/6/

In our case, we would like to stop the printing process if the signature fails. Does anyone have any tips on how we can achieve this?

This should be the default behavior already. If you change the above example from:

	return (resolve, reject) => {
-         reject("foo");
+         resolve("foo");
	};

... the call should succeed. There's a chance we're not handling the rejection properly, but the default behavior seems to be what's being described in this bug report where the print does not work when the signature fails.

@ThomasRutzer
Copy link
Author

ThomasRutzer commented Jul 10, 2024

Hi @tresf, thanks for your quick reply. Based on your example, if I would extend it with an .catch()–phrase, I'd actually expect it to log the error. But afaik, no error is thrown.

qz.security.setSignaturePromise(toSign => {
    return (resolve, reject) => {
        reject("foo");
    };
});

qz.websocket.connect().then(() => {
    return qz.printers.find();
}).then(printers => {
    console.log(printers);
}).catch(e => {
    console.log("signing error", e)
});

@tresf
Copy link
Contributor

tresf commented Jul 10, 2024

A signature failure is not actionable on the client-side, so it's not -- and has never been part of -- the connecting/printing/API call promise chain. This same statement is true for setCertificatePromise.

qz.security.setCertificatePromise((resolve, reject) => { reject("no"); });

Notice that the API calls do NOT reject.

@tresf
Copy link
Contributor

tresf commented Jul 10, 2024

I understand that this leaves the state of a print job in limbo. Certificates are public, can be provided in clear text and should never fail, so this is less of a concern. Signatures are often handed off to a controller so a failure can be a sign of another issue with the backend system. I'll bring this up to our JS developer to see if there's a way to throw the rejection up the call stack.

Pinging @akberenz.

@ThomasRutzer
Copy link
Author

Hi @tresf, thanks again. I'd suggest to leave this issue open then to see if @akberenz can figure something out?

@tresf
Copy link
Contributor

tresf commented Jul 11, 2024

This is available starting now with the snapshot version: https://raw.githubusercontent.com/qzind/tray/master/js/qz-tray.js

This will be released (e.g. to NPM) as qz-tray.js@2.2.4 in coordination with the future 2.2.4 release (probably sometime next week).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants