Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

Request for non-existing resource responds with 400 (runtime) #369

Closed
tripodsan opened this issue Jun 9, 2019 · 6 comments · Fixed by #370
Closed

Request for non-existing resource responds with 400 (runtime) #369

tripodsan opened this issue Jun 9, 2019 · 6 comments · Fixed by #370
Assignees
Labels
bug Something isn't working released

Comments

@tripodsan
Copy link
Contributor

Description
a request with a x-strain cookie of strain that does not exist returns a 400.

To Reproduce

$ curl -s -D - -b "X-Strain=foo"  -H X-Debug:true  https://www.project-helix.io/client/README.html
HTTP/2 400
access-control-allow-headers: Authorization, Origin, X-Requested-With, Content-Type, Accept, User-Agent
access-control-allow-methods: OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH
access-control-allow-origin: *
content-type: application/json
server: api-gateway/1.9.3.1
x-content-type-options: nosniff
x-frame-options: deny
x-openwhisk-activation-id: 413a39b4c9ac4362ba39b4c9ac136270
x-request-id: Z20CrNPBMQHXBgLM8VcVSIOBTJIWL3Fo
x-xss-protection: 1; mode=block
x-backend-name: 3t7g8apsfQlFyvuHIQn9e8--F_AdobeRuntime
x-cdn-request-id: 55c2fa79-d2b6-457e-a2b7-80d5ab92f9dd
accept-ranges: bytes
accept-ranges: bytes
date: Sun, 09 Jun 2019 12:48:10 GMT
via: 1.1 varnish
x-served-by: cache-tyo19923-TYO
x-cache: MISS
x-cache-hits: 0
x-timer: S1560084490.956233,VS0,VE309
vary: X-Debug,X-Strain,X-Request-Type
strict-transport-security: max-age=31536000
x-version: 121; src=121; cli=1.0.0; rev=online
x-url: /client/README.html
x-backend-url: /api/v1/web/helix/63a0124113294dd2e23b3fe85f79f280946112a6/html?owner=adobe&repo=project-helix.io&ref=master&path=/client/README.md&selector=&extension=html&strain=foo&rootPath=&params=
x-strain: foo
x-github-static-ref: @(null)
x-fulldirname: /client
x-action-root: helix/63a0124113294dd2e23b3fe85f79f280946112a6
x-orig-url: /client/README.html
x-repo-root-path:
x-trace: vcl_recv; hlx_recv_init; hlx_strain(cookie); hlx_block_recv; hlx_deny; hlx_allow; hlx_request_type(none); hlx_type_pipeline; hlx_owner; hlx_repo; hlx_ref; hlx_root_path; hlx_action_root; vcl_hash(www.project-helix.io/client/README.html); hlx_bereq,vcl_miss(www.project-helix.io/client/README.html); vcl_fetch(400: /client/README.html); vcl_deliver; hlx_headers_deliver; hlx_deliver_errors; hlx_deliver_static
content-length: 100

{
  "code": "Z20CrNPBMQHXBgLM8VcVSIOBTJIWL3Fo",
  "error": "Response is not valid 'message/http'."
}

The activation on wsk shows:

    "statusCode": 0,
    "response": {
        "status": "application error",
        "statusCode": 0,
        "success": false,
        "result": {
            "error": "StatusCodeError: 404 - \"404: Not Found\\n\"\n    at new StatusCodeError (/nodejsAction/XXLBElAn/node_modules/request-promise-core/lib/errors.js:32:15)\n    at Request.plumbing.callback (/nodejsAction/XXLBElAn/node_modules/request-promise-core/lib/plumbing.js:104:33)\n    at Request.RP$callback [as _callback] (/nodejsAction/XXLBElAn/node_modules/request-promise-core/lib/plumbing.js:46:31)\n    at Request.self.callback (/node_modules/request/request.js:185:22)\n    at Request.emit (events.js:189:13)\n    at Request.<anonymous> (/node_modules/request/request.js:1161:10)\n    at Request.emit (events.js:189:13)\n    at IncomingMessage.<anonymous> (/node_modules/request/request.js:1083:12)\n    at Object.onceWrapper (events.js:277:13)\n    at IncomingMessage.emit (events.js:194:15)"
        }
    },

Expected behavior
The server should respond with a 404.

@trieloff WDYT?

@tripodsan
Copy link
Contributor Author

tripodsan commented Jun 10, 2019

@tripodsan
Copy link
Contributor Author

the problem here is not the strain cookie - it only manifests here, because the root path of the strain is wrong. the underlying problem is that fetch fails.

eg:

$ curl -s  https://www.project-helix.io/client/foobar.html
{
  "code": "eUFFqI3JpLOgayxtg1nqWeZANIvqciAj",
  "error": "Response is not valid 'message/http'."
}

@tripodsan tripodsan self-assigned this Jun 10, 2019
@tripodsan
Copy link
Contributor Author

running the action locally shows the correct response:

$ node
> const html = require('./html.js');
> html.main({repo:'helix-pipeline', owner:'adobe', path:'not-exists.md'}).then(console.log).catch(console.error);
...
debug: Constructing HTML Pipeline
debug: fetching Markdown from https://raw.githubusercontent.com/adobe/helix-pipeline/master/not-exists.md
error: Could not find Markdown at https://raw.githubusercontent.com/adobe/helix-pipeline/master/not-exists.md
{ statusCode: 404,
  headers: {},
  body: '',
  error:
   'StatusCodeError: 404 - "404: Not Found\\n"\n    at new StatusCodeError (/Users/tripod/codez/helix/sandbox/hlxtest/.hlx/build/html/node_modules/request-promise-core/lib/errors.js:32:15)\n    at Request.plumbing.callback 

maybe something changed on openwhisk?

@tripodsan tripodsan changed the title Request with _wrong_ X-Strain cookie produces 400 Request for non-existing resource responds with 400 (runtime) Jun 10, 2019
@tripodsan
Copy link
Contributor Author

apparently, when the error is not set on the return value, the response is correct.

eg:

function main(...params) {
  return {
    headers: { 'Content-Type': 'text/plain' },
    statusCode: 404,
    error: 'not found',
    body: '',
  };
}

vs:

function main(...params) {
  return {
    headers: { 'Content-Type': 'text/plain' },
    statusCode: 404,
    body: '',
  };
}

@tripodsan
Copy link
Contributor Author

tripodsan commented Jun 10, 2019

as @alexkli pointed out:


Once there is one error field, it not only sees it as application error, but also drops all the other fields and only records the error field in the activation result.
That error field can be an object though, and should have its string message in another nested error field:

  error: {
    error: "did not work",
    statusCode: 500
  }
}

This is useful if you want a human readable error message and some programmatic error information such as codes.

whether the activation is an error or not is your choice I guess. I agree, for webactions a 4xx or 5xx http response should usually be a success result, no need for an application error. was just pointing out the options.


so when we use:

function main(params) {
  return {
    error: {
      headers: { 'Content-Type': 'text/html', 'X-Foo': 'bar' },
      statusCode: 404,
      error: 'not found'
    },
  };
}

then the result is as expected:

$ curl -D -  https://adobeioruntime.net/api/v1/web/tripod/default/test
HTTP/1.1 404 Not Found
Access-Control-Allow-Headers: Authorization, Origin, X-Requested-With, Content-Type, Accept, User-Agent
Access-Control-Allow-Methods: OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH
Access-Control-Allow-Origin: *
Content-Type: text/plain
Date: Mon, 10 Jun 2019 23:39:35 GMT
Server: api-gateway/1.9.3.1
X-Content-Type-Options: nosniff
X-Foo: bar
X-Frame-Options: deny
x-openwhisk-activation-id: adcbc496c8134a818bc496c8134a8158
X-Request-ID: bRlskyxwqkZqEb3OzoccN7RkfJAy83hN
X-XSS-Protection: 1; mode=block
Content-Length: 0
Connection: keep-alive

with:

$ wsk activation get -s adcbc496c8134a818bc496c8134a8158
activation result for '/tripod/test' (application error at 2019-06-11 08:39:35 +0900 JST)
{
    "error": {
        "error": "not found",
        "headers": {
            "Content-Type": "text/html",
            "X-Foo": "bar"
        },
        "statusCode": 404
    }
}

However, I agree with @alexkli, that we should not treat expected errors as application errors.

@tripodsan tripodsan transferred this issue from adobe/helix-cli Jun 10, 2019
@tripodsan tripodsan added the bug Something isn't working label Jun 10, 2019
adobe-bot pushed a commit that referenced this issue Jun 11, 2019
## [3.2.2](v3.2.1...v3.2.2) (2019-06-11)

### Bug Fixes

* **openwhisk:** never set the 'error' property of the response. ([5fc020a](5fc020a)), closes [#369](#369)
@adobe-bot
Copy link

🎉 This issue has been resolved in version 3.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants