search

esi / esi-issues

Issue tracking and feature requests for ESI
https://esi.evetech.net/
208 stars 23 forks source link

Deprecated warning header missing on error #1179

Open GoldenGnu opened 4 years ago

GoldenGnu commented 4 years ago

Bug

Deprecated warning header is missing on SSO error. Note: This may affect multiple endpoints (I have only tested the assets endpoint, because, I was wondering why my tests wasn't failing with the promotion).

Request

curl -X GET "https://esi.evetech.net/legacy/characters/1/assets/?datasource=tranquility&page=1" -H "accept: application/json"

Response

Status Code

401

Headers

access-control-allow-credentials: true
access-control-allow-headers: Content-Type,Authorization,If-None-Match,X-User-Agent
access-control-allow-methods: GET,HEAD,OPTIONS
access-control-allow-origin: *
access-control-expose-headers: Content-Type,Warning,ETag,X-Pages,X-ESI-Error-Limit-Remain,X-ESI-Error-Limit-Reset
access-control-max-age: 600
allow: GET,HEAD,OPTIONS
content-language: en-us
content-length: 38
content-type: application/json; charset=utf-8
date: Sun, 15 Mar 2020 18:35:05 GMT
strict-transport-security: max-age=31536000
vary: Accept-Language  x-esi-error-limit-remain: 99  x-esi-error-limit-reset: 55  x-firefox-spdy: h2

Body

{ "error": "authorization not provided" }

Expected

Expected warning header because the endpoint version is deprecated

warning: 299 - This route is deprecated

Checklist

Check all boxes that apply to this issue:

jamesrobb commented 3 years ago

Thanks for reporting this :)

I can definitely see why one might expect the behaviour you expect, but this is in fact working as intended. When SSO authorization fails, your request is stopped at the first "gate" so to say, that is, the request never makes it far enough that the deprecated endpoint is actually hit.

CarbonAlabel commented 3 years ago

Quoting https://developers.eveonline.com/blog/article/breaking-changes-and-you:

Once an alt-route is only available on /legacy (or if you call /legacy directly), your app will start receiving "warning" headers with every payload, even on 401 unauthorized errors. If your app is running through a CI system, one of your tests should be "blindly call every route I use and see if I get a warning header in the return". This will tell you if any of the routes you are using have been moved to legacy, so you can update your app back to latest.

That blog post is over 3 years old, but there was never any announcement of a change in this behavior, so I find the statement of "working as intended" a bit suspicious. I don't think I need to point out how impractical including valid auth in a CI process would be.

jamesrobb commented 3 years ago

Quoting https://developers.eveonline.com/blog/article/breaking-changes-and-you:

Once an alt-route is only available on /legacy (or if you call /legacy directly), your app will start receiving "warning" headers with every payload, even on 401 unauthorized errors. If your app is running through a CI system, one of your tests should be "blindly call every route I use and see if I get a warning header in the return". This will tell you if any of the routes you are using have been moved to legacy, so you can update your app back to latest.

That blog post is over 3 years old, but there was never any announcement of a change in this behavior, so I find the statement of "working as intended" a bit suspicious. I don't think I need to point out how impractical including valid auth in a CI process would be.

Right. We will look into this further.

Internal issue: TTC-3839