search

jsattler / go-comdirect

Go module and CLI tool to interact with the comdirect REST API
Apache License 2.0
42 stars 9 forks source link

Add CLI flag to specify output file #44

Open aa-ko opened 8 months ago

aa-ko commented 8 months ago

Hi! I ran into an issue when trying to use the CLI in a script. If the tool generates an error message while waiting on the 2FA authentication to complete, this error will be written to stdout along with the (CSV/JSON) output from the comdirect API. In my case, the tool is called like this: # comdirect depot position <id> -f csv > depot.csv This sometimes results in an invalid CSV file, where the first line contains the error message.

This is why I propose the addition of a new -o / --output flag, that will always create a valid output file while still printing any log message to stdout/stderr.

My implementation of this is working fine and I'm happy open a PR with my changes after some more testing and if this is something you'd like to support :)

jsattler commented 8 months ago

Hi @aa-ko, first of all that sounds like a good idea. Can you share a little bit more about the error that you are getting during the 2FA? Maybe we need to improve the error handling in this case as well. Also, what would be a valid output in case an error occurs? Thanks for your interest in the project and for submitting the issue.

aa-ko commented 8 months ago

Hi @jsattler, thanks for the quick reply!

Let me rephrase this a little bit. If the timeout is reached before I am able to authenticate the request, a context deadline exceeded message will be written to the file: 2024/01/12 19:56:26 context deadline exceeded If the 2FA token has expired and I have to authenticate again, I will have this message at the top of my file: Your session expired. Please open the comdirect photoTAN app to validate a new session.

The second case will always be a problem, even if I am fast enough to re-authenticate in time, because the info message will be written anyways if the token is no longer valid.

As for improving the error handling, I don't think there is any need to change the current code. All messages I had problems with are correct and useful, they are just at the wrong place, if I happen to redirect stdout to a file.

In my opinion, the correct behavior would be to simply not write to file if an error occurs and to still print all messages to stdout/stderr. For usage in a script, this can be further improved by returning with specific error codes.