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

fix(pdk) kong.response.get_source() to return error on plugin throws runtime exception #8599

Merged
merged 5 commits into from
Apr 7, 2022

Conversation

ms2008
Copy link
Contributor

@ms2008 ms2008 commented Mar 28, 2022

Summary

If plugin that throws runtime exception on access phase, kong.response.get_source() should returned error instead of exit.

FTI-3200

@CLAassistant
Copy link

CLAassistant commented Mar 28, 2022

CLA assistant check
All committers have signed the CLA.

@ms2008 ms2008 requested a review from kikito March 28, 2022 14:50
Copy link
Member

@dndx dndx left a comment

Choose a reason for hiding this comment

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

Looks reasonable, can we add a test case using severless-plugin?

@andreleoni
Copy link
Contributor

This change context should be by plugin, instead of all application initializer?

In my opinion, if a error was received, the application should exit, instead of only log an error, should be dangerous only log an error...

🤔

@ms2008
Copy link
Contributor Author

ms2008 commented Mar 29, 2022

This change context should be by plugin, instead of all application initializer?

In my opinion, if a error was received, the application should exit, instead of only log an error, should be dangerous only log an error...

🤔

In access phase we need to run through the whole plugin iterator to get list of plugins for the request. You cannot exit it while looping. We need to collect the plugins even in case of an error.

@pull-request-size pull-request-size bot added size/M and removed size/XS labels Apr 6, 2022
@dndx dndx force-pushed the fix/pdk-response-source branch from 3ffb487 to e5757a2 Compare April 7, 2022 07:15
@dndx dndx merged commit 0b0c88c into master Apr 7, 2022
@dndx dndx deleted the fix/pdk-response-source branch April 7, 2022 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants