Skip to content

Conversation

@ywkaras
Copy link
Contributor

@ywkaras ywkaras commented Feb 4, 2022

Adds Au test with plugin forcing various errors on various hooks, with default body
or set body.

@ywkaras ywkaras self-assigned this Feb 4, 2022
@ywkaras ywkaras changed the title Make sure, when plugins for a GET request to fail, there is always a response body. Make sure, when plugins cause a GET request to fail, there is always a response body. Feb 4, 2022
@ywkaras ywkaras added this to the 10.0.0 milestone Feb 4, 2022
@ywkaras ywkaras force-pushed the 403_post_remap branch 2 times, most recently from f709eb8 to 093aff5 Compare February 4, 2022 19:35
@ywkaras
Copy link
Contributor Author

ywkaras commented Feb 8, 2022

@ywkaras
Copy link
Contributor Author

ywkaras commented Feb 9, 2022

[approve ci centos]

…response body.

Adds Au test with plugin forcing various errors on various hooks, with default body
or set body.
@apache apache deleted a comment from bneradt May 17, 2022
@apache apache deleted a comment from bneradt May 17, 2022
@apache apache deleted a comment from ezelkow1 May 17, 2022
//
// This class and the base class are pseudo-namespaces, have only static member functions (no data members).
// The manager data cannot simply be in the base class, because the (static member) continuation function that
// runs on the TXN_CLOSE hook would not be able to access it.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment improvement is not directly related to this PR.

#include <ts/experimental.h>
#include <tscore/ink_defs.h>
#include <tscpp/util/PostScript.h>
#include <tscpp/util/TextView.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed double quotes to braces to make it clear these are not subdirs under xdebug.

#include <tscore/ink_defs.h>
#include <tscpp/util/PostScript.h>
#include <tscpp/util/TextView.h>
#include <tscpp/api/Cleanup.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No real change to xdebug, just from this header to make it available to all plugins.

const char *reason = http_hdr_reason_lookup(s->http_return_code);
;
build_response(s, &s->hdr_info.client_response, s->client_info.http_version, s->http_return_code, reason ? reason : "Error");
build_error_response(s, s->http_return_code, reason ? reason : "Error", nullptr);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only real code change. Most of this PR is to add an Au test.

@ywkaras
Copy link
Contributor Author

ywkaras commented Jun 7, 2022

Is this an incompatible change? Since plugin which may have generated empty response bodies in some cases will not generate non-empty response bodies?

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

@github-actions github-actions bot added the Stale label Sep 6, 2022
@ywkaras
Copy link
Contributor Author

ywkaras commented Sep 12, 2022

[approve ci autest]

@ywkaras ywkaras closed this Sep 12, 2022
@ywkaras ywkaras reopened this Sep 12, 2022
@ezelkow1
Copy link
Member

[approve ci autest]

@ywkaras ywkaras marked this pull request as draft September 12, 2022 23:50
@ywkaras
Copy link
Contributor Author

ywkaras commented Sep 13, 2022

This was an ask from YCPI Prod. But, no point in fixing it if it's going to continue to rot in the Sun.

@github-actions github-actions bot removed the Stale label Sep 13, 2022
@ywkaras ywkaras closed this Oct 11, 2022
@zwoop zwoop removed this from the 10.0.0 milestone Jan 17, 2023
@zwoop zwoop added Stale and removed Dormant labels Feb 3, 2023
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.

4 participants