[GitHub] [apisix] spacewander commented on a change in pull request #4164: feat: add 50x html for error page

[GitBox]

spacewander commented on a change in pull request #4164:
URL: https://github.com/apache/apisix/pull/4164#discussion_r629899826



##
File path: .gitignore
##
@@ -74,3 +74,4 @@ ci/openwhisk-utilities/
 # release tar package
 *.tgz
 release/*
+html/*

Review comment:
   To make the license check pass, also need to change 
https://github.com/apache/apisix/blob/504cd1b0966880101b3e2582cfb1cf65f23a7863/ci/ASF-Release.cfg#L82

##
File path: t/APISIX.pm
##
@@ -426,6 +429,18 @@ _EOC_
 more_clear_headers Date;
 }
 
+location \@50x.html {
+set \$error_page 'true';

Review comment:
   Better to use `from_error_page` instead




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [apisix] spacewander commented on a change in pull request #4164: feat: add 50x html for error page

[GitBox]

spacewander commented on a change in pull request #4164:
URL: https://github.com/apache/apisix/pull/4164#discussion_r629796005



##
File path: apisix/cli/ops.lua
##
@@ -605,6 +611,9 @@ local function start(env, ...)
 local cmd_logs = "mkdir -p " .. env.apisix_home .. "/logs"
 util.execute_cmd(cmd_logs)
 
+local cmd_html = "mkdir -p " .. env.apisix_home .. "/html"

Review comment:
   Need to add it to gitignore: 
https://github.com/apache/apisix/blob/504cd1b0966880101b3e2582cfb1cf65f23a7863/.gitignore#L48




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [apisix] spacewander commented on a change in pull request #4164: feat: add 50x html for error page

[GitBox]

spacewander commented on a change in pull request #4164:
URL: https://github.com/apache/apisix/pull/4164#discussion_r629776898



##
File path: html/50x.html
##
@@ -0,0 +1,38 @@

[GitHub] [apisix] spacewander commented on a change in pull request #4164: feat: add 50x html for error page

[GitBox]

spacewander commented on a change in pull request #4164:
URL: https://github.com/apache/apisix/pull/4164#discussion_r629054652



##
File path: apisix/cli/ngx_tpl.lua
##
@@ -617,6 +642,17 @@ http {
 proxy_pass $upstream_mirror_host$request_uri;
 }
 {% end %}
+
+location = /50x.html {

Review comment:
   So the log phase might get a rewritten URL instead of the original one? 
This is not good news...




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [apisix] spacewander commented on a change in pull request #4164: feat: add 50x html for error page

[GitBox]

spacewander commented on a change in pull request #4164:
URL: https://github.com/apache/apisix/pull/4164#discussion_r628984645



##
File path: apisix/init.lua
##
@@ -620,6 +619,15 @@ end
 
 function _M.http_log_phase()
 local api_ctx = common_phase("log")
+if not api_ctx then
+local ctx = ctxdump.apply_ngx_ctx(ngx_var.ctx_ref)

Review comment:
   Why not reapply the ctx in `http_header_filter_phase`?

##
File path: apisix/cli/ngx_tpl.lua
##
@@ -617,6 +642,17 @@ http {
 proxy_pass $upstream_mirror_host$request_uri;
 }
 {% end %}
+
+location = /50x.html {

Review comment:
   Would it be better to use `named location`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org