Skip to content

Error message not returned to client, get empty JSON instead #1597

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

Closed
traturne opened this issue Jun 27, 2020 · 6 comments
Closed

Error message not returned to client, get empty JSON instead #1597

traturne opened this issue Jun 27, 2020 · 6 comments

Comments

@traturne
Copy link

traturne commented Jun 27, 2020

Issue Description

See the following code:

if err := c.Bind(req); err != nil {
		msg := "Unable to bind DataAPISearchProductsRequest: " + err.Error()
		return echo.NewHTTPError(http.StatusBadRequest, errors.New(msg))
	}

I do not see the error message in the response from the server for the following invalid request.

PAYLOAD=$(cat <<-END
{
  query": "wrench"
}
END
)

curl -v -H "Content-Type: application/json" \
 -X POST http://localhost:8080/search_products -d $PAYLOAD
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying ::1:8080...
* TCP_NODELAY set
* Connected to localhost (::1) port 8080 (#0)
> POST /search_products HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.65.3
> Accept: */*
> Content-Type: application/json
> Content-Length: 22
>
* upload completely sent off: 22 out of 22 bytes
* Mark bundle as not supporting multiuse
< HTTP/1.1 400 Bad Request
< Content-Type: application/json; charset=UTF-8
< Date: Sat, 27 Jun 2020 03:32:24 GMT
< Content-Length: 3
<
{}
* Connection #0 to host localhost left intact

The error is logged just fine:

{"time":"2020-06-26T20:32:01.452707-07:00","id":"","remote_ip":"::1","host":"localhost:8080","method":"POST","uri":"/search_products","user_agent":"curl/7.65.3","status":400,"error":"code=400, message=Unable to bind DataAPISearchProductsRequest: code=400, message=Syntax error: offset=5, error=invalid character 'q' looking for beginning of object key string","latency":41355283633,"latency_human":"41.355283633s","bytes_in":22,"bytes_out":3}

Expected behaviour

I expect to see the error message int he response.

Actual behaviour

I do not see the error message in the response.

Steps to reproduce

I'm not doing anything fancy here, just trying to use the default HTTP error handler.

Version/commit

v3.3.10-dev

@sm4ll-3gg
Copy link

sm4ll-3gg commented Jul 28, 2020

Hello!
You see that behavior because returned by errors.New error couldn't be marshaled in JSON correctly. It's not a bug in echo.

Look at this example: https://play.golang.org/p/d8eStiK3z1R

To fix this you can just pass a string to echo.NewHTTPError as a message instead of result of errors.New calling.

Hope it'll help you ;)

@lammel
Copy link
Contributor

lammel commented Sep 4, 2020

I guess this is fixed in v4 where the error is unwrapped actually.

@stale
Copy link

stale bot commented Nov 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 7, 2020
@lammel lammel added stale Marked as stale for auto-closing and removed wontfix labels Mar 9, 2021
@lammel
Copy link
Contributor

lammel commented Mar 9, 2021

An error with incorrect handling of errors was fixed for v4.2.
So this should work with the latest release. Please close/confirm @traturne

@stale stale bot removed the stale Marked as stale for auto-closing label Mar 9, 2021
@Meowzz95
Copy link

Meowzz95 commented Nov 15, 2021

I'm using github.com/labstack/echo/v4 v4.6.1

return c.JSON(401,body)
This returns empty object {} when body is error type, is this expected?

image
Error is generated by errors.New


Edit:

type AuthError struct {
	Message string
}

I tried to use a custom error type with a public member, it works. However, from the above discussion, it seems that errors.New should work too. Did I misunderstand something?

@lammel
Copy link
Contributor

lammel commented Dec 2, 2022

This has probably been fixed by PR #1666 (see also #1477).
Closing.

@lammel lammel closed this as completed Dec 2, 2022
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