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

ngx.exit(ngx.HTTP_NOT_FOUND) still returns 200 #15

Closed
scalp42 opened this issue Mar 11, 2013 · 11 comments
Closed

ngx.exit(ngx.HTTP_NOT_FOUND) still returns 200 #15

scalp42 opened this issue Mar 11, 2013 · 11 comments

Comments

@scalp42
Copy link

scalp42 commented Mar 11, 2013

Hi Yichun,

Thanks a lot for all those modules !

I tried to run your example and can't figure how to actually returns a 404 if the key is not found (hence res == null in local res, err = red:get(ngx.var.key_that_does_not_exist).

Snippet:

    if res == ngx.null then
      ngx.status = ngx.HTTP_NOT_FOUND
      ngx.say("key not found.")
      return ngx.status
    end

    ngx.say("result: ", res)

Also tried:

    if res == ngx.null then
      ngx.status = ngx.HTTP_NOT_FOUND
      ngx.say("key not found.")
      ngx.exit(ngx.status)
    end

    ngx.say("result: ", res)

Output:

$> curl -I redis/key_exists
HTTP/1.1 200 OK
Server: ngx_openresty/1.2.7.1
result: "the key exists!"

$> curl -I redis/key_does_not_exist
HTTP/1.1 200 OK
Server: ngx_openresty/1.2.7.1
key not found.

Any idea by any chance ?

@agentzh
Copy link
Member

agentzh commented Mar 11, 2013

Hello!

On Mon, Mar 11, 2013 at 12:44 AM, Anthony Scalisi
notifications@github.comwrote:

Thanks a lot for all those modules !

You're welcome :)

I tried to run your example and can't figure how to actually returns a 404
if the key is not found (hence res == null in local res, err =
red:get(ngx.var.key_that_does_not_exist).

Here is a complete example:

location /t {
    content_by_lua '
        local redis = require "resty.redis"
        local red = redis:new()

        red:set_timeout(1000) -- 1 sec

        local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT)
        if not ok then
            ngx.say("failed to connect: ", err)
            return
        end

        local res, err = red:get("key_not_existed")

        if err then
            ngx.say("failed to get dog: ", err)
            return
        end

        if res == ngx.null then
            ngx.status = 404
            ngx.say("key not found.")
            ngx.exit(404)
        end

        ngx.say("res: ", res)
    ';
}

Tested on my side:

$ curl -i localhost:8080/t
HTTP/1.1 404 Not Found
Server: nginx/1.2.7
Date: Mon, 11 Mar 2013 18:35:18 GMT
Content-Type: text/plain
Transfer-Encoding: chunked
Connection: keep-alive

key not found.

Looking good, huh?

Snippet:

if res == ngx.null then
  ngx.status = ngx.HTTP_NOT_FOUND
  ngx.say("key not found.")
  return ngx.status
end

ngx.say("result: ", res)

Well, the return value is actually ignored here.

Also tried:

if res == ngx.null then
  ngx.status = ngx.HTTP_NOT_FOUND
  ngx.say("key not found.")
  ngx.exit(ngx.status)
end

ngx.say("result: ", res)

This one should work if you do not send a header before this snippet is
reached (calling ngx.print and ngx.say also triggers response header
sending). You can check out your Nginx's error.log file to be sure.

curl redis/key_does_not_exist
HTTP/1.1 200 OK
Server: ngx_openresty/1.2.7.1
key not found.

Looks like the response header was indeed sent before you tested the redis
result.

Regards,
-agentzh

@scalp42
Copy link
Author

scalp42 commented Mar 11, 2013

Thanks for the reply @agentzh.

I'm still getting a 200 OK on a key not found in Redis (based on $uri).

Please have a look at this snippet, this is the full code:

https://gist.github.com/scalp42/5137764

I'm making sure to strip the leading / from the $uri.

You can test in Redis using:

redis-cli set exists-json FOUND

And then using curl:

$> curl localhost:8080/exists
Requested: exists-json
FOUND (200 OK)

$> curl localhost:8080/
Requested: -json
($json = null-json, so 404)

$> curl localhost:8080/wrongkey
Requested: wrongkey-json
key not found (wrongkey-json, but still 200 OK)

Thanks a lot in advance!

@scalp42
Copy link
Author

scalp42 commented Mar 11, 2013

I just figured it out based on your comments. I indeed call ngx.say at the very top of the lua script, bypassing the later call ngx.exit(ngx.status).

UPDATED: still getting 200 OK for some reason even removing that top ngx.say

I see the problem in error.log:

[error] 25038#0: *1 attempt to set status 404 via ngx.exit after sending out the response status 200

but I removed any ngx.say call.

@scalp42
Copy link
Author

scalp42 commented Mar 11, 2013

I figured it out even though it's not clear to me:

200 OK:

if res == ngx.null then
  ngx.say("key not found.")
  ngx.exit(404)
end

404:

if res == ngx.null then
  ngx.status = 404
  ngx.say("key not found.")
  ngx.exit(404)
end

I'm confused regarding ngx.exit.

Per the doc http://wiki.nginx.org/HttpLuaModule#ngx.exit :

The status argument can be ngx.OK, ngx.ERROR, ngx.HTTP_NOT_FOUND, ngx.HTTP_MOVED_TEMPORARILY, or other HTTP status constants.

ngx.exit(501)

It looks like ngx.status need to be set up:

WORKING:
if res == ngx.null then
    ngx.status = 404
    ngx.say("key not found.")
    ngx.exit(404) ## Is the 404 needed here ?
end

NOT WORKING (missing ngx.status):
if res == ngx.null then
  ngx.say("key not found.")
  return ngx.exit(ngx.HTTP_NOT_FOUND)
end

@agentzh
Copy link
Member

agentzh commented Mar 11, 2013

Hello!

By default, ngx_lua output response data in a non-buffered manner (in
order to save memory and increase I/O and CPU overlapping).

When you call ngx.say() for the first time, the response header will
automatically be sent right away if it is not sent yet. After that,
even if you call ngx.exit(404), the response header cannot be altered
because it has already been sent out (unless you have a time machine).

Consider the following minimal example:

location = /t {
    content_by_lua '
        ngx.say("hello")
        ngx.exit(404)
    ';
}

Accessing /t will yield

HTTP/1.1 200 OK
Server: nginx/1.2.7
Date: Mon, 11 Mar 2013 22:35:34 GMT
Content-Type: text/plain
Transfer-Encoding: chunked
Connection: keep-alive

hello

Why 200? Because the response header with 200 (the default status
code) is sent out automatically as soon as we call ngx.say() for the
1st time in Lua.

You can change the status code by setting ngx.status before sending
the response header (and before calling ngx.say, because it
automatically triggers response header sending). That is,

location = /t {
    content_by_lua '
        ngx.status = 404
        ngx.say("hello")
        ngx.exit(404)
    ';
}

This is documented in the manual for ngx.say and ngx.print:

http://wiki.nginx.org/HttpLuaModule#ngx.print

To quote: "emits arguments concatenated to the HTTP client (as
response body). If response headers have not been sent, this function
will send headers out first and then output body data."

Regards,
-agentzh

@agentzh
Copy link
Member

agentzh commented Mar 11, 2013

Hello!

On Mon, Mar 11, 2013 at 3:39 PM, agentzh agentzh@gmail.com wrote:

Consider the following minimal example:

location = /t {
    content_by_lua '
        ngx.say("hello")
        ngx.exit(404)
    ';
}

Accessing /t will yield

HTTP/1.1 200 OK
Server: nginx/1.2.7
Date: Mon, 11 Mar 2013 22:35:34 GMT
Content-Type: text/plain
Transfer-Encoding: chunked
Connection: keep-alive

hello

Why 200? Because the response header with 200 (the default status
code) is sent out automatically as soon as we call ngx.say() for the
1st time in Lua.

Also, if you check out your nginx's error.log file (please always do
that to save your time!), then you should see the following error
message:

[error] 12251#0: *1 attempt to set status 404 via ngx.exit after

sending out the response status 200

So your ngx.exit() call just complaints about the fact that a response
header with a differents status code has already been sent out.

Regards,
-agentzh

@scalp42
Copy link
Author

scalp42 commented Mar 12, 2013

Thanks for the precisions @agentzh, much appreciated.

👍

@scalp42 scalp42 closed this as completed Mar 12, 2013
@HoldSoulSkip
Copy link

the same situation happenes to me ,this is a strange thing~

@agentzh
Copy link
Member

agentzh commented Mar 21, 2016

@HoldSoulSkip You must have sent out the request header earlier (maybe implicitly by calling ngx.print or ngx.say earlier in your code paths). We don't have a time machine so the status code set by a later ngx.exit() call cannot take effect. Please ensure you have set ngx.status = code before your first response output call.

@HoldSoulSkip
Copy link

@agentzh Thank you very much i will try a soon later

@bum
Copy link

bum commented Jul 8, 2020

You should set response status before "say"

local function failed(log, err)
	ngx.status = ngx.HTTP_INTERNAL_SERVER_ERROR
	ngx.say(log)
	ngx.log(ngx.ERR, log, err)
	return ngx.exit(ngx.status)
end

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

No branches or pull requests

4 participants