joshbalfour / node-cognitive-services

Microsoft® Cognitive Services SDK for Node.JS
https://www.microsoft.com/cognitive-services
MIT License
105 stars 45 forks source link

202 response and operation-location header #38

Closed v-geberr closed 6 years ago

v-geberr commented 6 years ago

Expected Behavior

(commonService.js, line 206) Body is returned if it exists.

Actual Behavior

Body is ignored if response header operation-location is returned.

If LUIS returns a 202, and a body, either ignore the operation-location header or return both the operation-header and the body.

miparnisari commented 6 years ago

The reason this was done this way was because until now i hadn't seen an endpoint that returned both the header and a body. Maybe we should return an object like

{
  "operationLocation": "xxxxxx",
  "body": {
      "key": "value"
  }
}

If the API doesn't return a body we can omit the property.

What's the endpoint that returns both things so I can test this?

v-geberr commented 6 years ago

Create an app via either import or customprebuiltdomains.

import https://westus.dev.cognitive.microsoft.com/docs/services/5890b47c39e2bb17b84a55ff/operations/5890b47c39e2bb052c5b9c31

customprebuiltdomains https://westus.dev.cognitive.microsoft.com/docs/services/5890b47c39e2bb17b84a55ff/operations/59104e515aca2f0b48c76be5

v-geberr commented 6 years ago

I need the numeric http response code. Could the project have a setting to turn on more features such returning the entire response?

v-geberr commented 6 years ago

I wind up changing core files in order to get the information I need. But this should be allowed with config settings.

I would like to create a config testing that, when turned on, would either console.log or file log items such as the full url and the verb (https://westus.api.cognitive.microsoft.com/luis/api/v2.0/apps/3576ad9d-16af-4131-a2fa-f2692f295bde/versions - GET) and the actual response number and text.

It could be off by default and could live in the config.json. Would you take a PR for this?

miparnisari commented 6 years ago

It could be off by default and could live in the config.json. Would you take a PR for this?

Of course :)