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

Requests were getting hanged with 'secondGuessSourceContentType' for empty images #160

Merged
merged 2 commits into from
Sep 21, 2021

Conversation

paras20xx
Copy link
Collaborator

No description provided.

@papandreou
Copy link
Owner

Hey Priyank, long time! 🤗

Nice find, but it's curious that the test times out on CI? That kinda indicates that it's not fixed by this patch? 🤔

@paras20xx
Copy link
Collaborator Author

@papandreou
hi 😄
Yes ... I was looking into that ... on my local (with Node JS v14.17.6), the tests were working, realized the issue from CI report.

Also, as per the CI logs, even npm install seems to be failing, which is weird. I'll try to investigate if some sub-dependency is causing that issue.

@papandreou
Copy link
Owner

Thanks for giving this project a long overdue health check 👨‍⚕️

…empty images - Updated fix for Node JS 12 or older
@paras20xx
Copy link
Collaborator Author

@papandreou
The issue with npm install was apparently some weird/intermittent issue. I tested with some reverts and re-reverts and it eventually went away.

The issue for older versions of Node appeared to be going deeper into streams (related to "impro" package). But, I was able to implement a minor workaround in the fix part.

The "Coverage" test is failing in the "upload" step. I'm not sure if it is supposed to work or not. For your reference: https://github.com/papandreou/express-processimage/runs/3662154920

Kindly merge if the changes look alright :-)

@papandreou papandreou merged commit 5011979 into master Sep 21, 2021
@papandreou
Copy link
Owner

Released in 10.2.1, thanks 👍

@paras20xx
Copy link
Collaborator Author

Thank you 😁

@paras20xx paras20xx deleted the secondGuessSourceContentType-for-empty-images branch September 21, 2021 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants