csingley / ofxtools

Python OFX Library
Other
301 stars 68 forks source link

ofxtools hanging on poor FI behavior #157

Open aclindsa opened 2 years ago

aclindsa commented 2 years ago

I've seen ofxtools hang a few times (with different financial institutions), and when I Ctrl-C it, it is in the following loop:

https://github.com/csingley/ofxtools/blob/8455c43a59f3ed17b72862ca6021240f33b099cd/ofxtools/header.py#L265

This behavior seems to be transient (the same FI may not cause it on other days...). This is from a call to client.request_statements.

From adding some debug prints, it appears that the FIs in question are returning endless empty lines (lovely)

Maybe this could fail more gracefully (like a limit on the max number of iterations of this loop so it doesn't become infinite for poorly-behaved FIs)?

csingley commented 2 years ago

Yeah that is not the finest piece of production quality code I have ever written. Surely I can find a more refined algo for discriminating the header from the body.

In other news, I may soon have need to become proficient in Go. How would you feel about rank novices clawing through your codebase and submitting ill conceived PRs riddled with obvious bugs?

aclindsa commented 2 years ago

In other news, I may soon have need to become proficient in Go. How would you feel about rank novices clawing through your codebase and submitting ill conceived PRs riddled with obvious bugs?

GO for it (haha heh ha ... ha?), that's half of what this "open source" stuff is all about.

Just don't assume that you'll learn the most idiomatic Go from looking at my code...

aclindsa commented 2 years ago

Another wrinkle: ofxgo seems to be able to successfully download OFX from the offending FI... So there may be another layer to this.

csingley commented 2 years ago

Maybe this could fail more gracefully (like a limit on the max number of iterations of this loop so it doesn't become infinite for poorly-behaved FIs)?

I'm not sure there's a better solution than this for neverending file descriptors, and it's simple to code. What I'm wrestling with is how to write the unit test. I haven't forgotten about this.

aclindsa commented 2 years ago

I'm not sure there's a better solution than this for neverending file descriptors, and it's simple to code.

I actually have a version of this locally and it "works", though its not as clean as might be ideal. I played around yesterday with trying to figure out if I could catch this condition higher in the call hierarchy (closer to where the request is made) but did not yet find a generic way to detect that the response was 0 bytes.

thehesiod commented 3 months ago

I just hit this, pre-readline you should be able to cache to buffer position via .tell() and if it hasn't moved after a readline you can consider it EOF