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

Prevent throwing another exception if the request fails eg connection reset #715

Merged

Conversation

davidvanlaatum
Copy link
Contributor

@davidvanlaatum davidvanlaatum commented Aug 2, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

#714 fix to prevent calling close on nil object

@cliffano @zlx

@davidvanlaatum davidvanlaatum changed the title prevent throwing another exception if the request fails eg connection reset #714 prevent throwing another exception if the request fails eg connection reset Aug 2, 2018
@@ -238,7 +238,7 @@ module {{moduleName}}
tempfile.write(chunk)
end
request.on_complete do |response|
tempfile.close
tempfile.close if tempfile
@config.logger.info "Temp file written to #{tempfile.path}, please copy the file to a proper folder "\
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note shouldn't there be a way to turn this off?

@wing328
Copy link
Member

wing328 commented Aug 2, 2018

@davidvanlaatum the fix looks good to me.

For @config.logger.info, I think you should be able to config the logger to not log anything for "info"

@davidvanlaatum
Copy link
Contributor Author

disabling logging of info doesn't feel like the right way to do it. Feels more like a dev hint that should go away once its been seen

@davidvanlaatum
Copy link
Contributor Author

davidvanlaatum commented Aug 2, 2018

for the build failure should I remove the 123_ that got added in a bunch of places? I assume thats a different problem

@wing328
Copy link
Member

wing328 commented Aug 2, 2018

for the build failure should I remove the 123_ that got added in a bunch of places? I assume thats a different problem

No worry. Let me fix that.

@wing328
Copy link
Member

wing328 commented Aug 2, 2018

UPDATE: I've filed #719 to fix the Travis CI failure.

@wing328 wing328 added this to the 3.2.0 milestone Aug 2, 2018
@wing328 wing328 merged commit a258cf3 into OpenAPITools:master Aug 2, 2018
@wing328
Copy link
Member

wing328 commented Aug 2, 2018

@davidvanlaatum The fix has been merged into master. Thanks for your contribution.

@wing328 wing328 changed the title #714 prevent throwing another exception if the request fails eg connection reset Prevent throwing another exception if the request fails eg connection reset Aug 6, 2018
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
…ils eg connection reset (OpenAPITools#715)

* prevent throwing another exception if the request fails eg connection reset

* prevent throwing another exception if the request fails eg connection reset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants