microsoftgraph / msgraph-sdk-javascript

Microsoft Graph client library for JavaScript
https://graph.microsoft.com
MIT License
758 stars 231 forks source link

Specific endpoint call fails by fetch of node v18 #999

Open bigwheel opened 2 years ago

bigwheel commented 2 years ago

Bug Report

Prerequisites

For more information, see the CONTRIBUTING guide.

Description

The fetch of node v18 introduces a bug to msgraph-sdk-javascript.

This is simple code.

  public async getApplications(): Promise<any> {
    return await this.client
      .api(`/applications`)
      .get();
  }

  public async getApplication(): Promise<any> {
    return await this.client
      .api(`/applications/deadbeef-xxxx-xxxx-xxxx-xxxxxxxxxxxx`)
      .get();
  }

This works well with node v16. However, in v18 it outputs following error.

❯ npx ts-node script/nishida-sample.ts
https://graph.microsoft.com/v1.0/applications/deadbeef-xxxx-xxxx-xxxx-xxxxxxxxxxxx
(node:99543) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/@microsoft/microsoft-graph-client/src/GraphError.ts:61
                super(message || (baseError && baseError.message));
  ^
GraphError: Invalid language specified.
    at new GraphError (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/@microsoft/microsoft-graph-client/src/GraphError.ts:61:3)
    at Function.GraphErrorHandler.constructErrorFromResponse (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/@microsoft/microsoft-graph-client/src/GraphErrorHandler.ts:76:18)
    at Function.<anonymous> (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/@microsoft/microsoft-graph-client/src/GraphErrorHandler.ts:102:31)
    at step (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/tslib/tslib.js:144:27)
    at Object.next (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/tslib/tslib.js:125:57)
    at /Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/tslib/tslib.js:118:75
    at new Promise (<anonymous>)
    at __awaiter (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/tslib/tslib.js:114:16)
    at Function.GraphErrorHandler.getError (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/@microsoft/microsoft-graph-client/lib/src/GraphErrorHandler.js:85:38)
    at GraphRequest.<anonymous> (/Users/kazufumi.nishida/code/_infra_others/graph-api-sandbox/node_modules/@microsoft/microsoft-graph-client/src/GraphRequest.ts:391:55) {
  statusCode: 400,
  code: 'Request_BadRequest',
  requestId: '329b85ba-dc7a-4712-b9b7-89e3907972c8',
  date: 2022-10-11T18:44:18.000Z,
  body: '{"code":"Request_BadRequest","message":"Invalid language specified.","innerError":{"date":"2022-10-12T03:44:18","request-id":"329b85ba-dc7a-4712-b9b7-89e3907972c8","client-request-id":"2729c80a-94c7-748f-31c2-fe56bb594a84"}}'
}

Also, it works once more with --no-experimental-fetch.

❯ NODE_OPTIONS=--no-experimental-fetch npx ts-node script/nishida-sample.ts
https://graph.microsoft.com/v1.0/applications/deadbeef-xxxx-xxxx-xxxx-xxxxxxxxxxxx
{"@odata.context":"https://graph.microsoft.com/v1.0/$metadata#applications/$entity","id":"
...
}

Steps to Reproduce

  1. Use node v18
  2. Setup ordinally (https://github.com/microsoftgraph/msgraph-sdk-javascript/tree/099afbe343cd279f218036a83eb2831ad065f46d#getting-started)
  3. Access v1.0/applications/deadbeef-xxxx-xxxx-xxxx-xxxxxxxxxxxx (use existing application id in your tenant)

Expected behavior: [What you expected to happen]

Return application property

Actual behavior: [What actually happened]

GraphError: Invalid language specified.

Additional Context

I was struggling 13 hours because new fetch of v18 works well except GET v1.0/applications/deadbeef-xxxx-xxxx-xxxx-xxxxxxxxxxxx endpoint. E.g, GET v1.0/applications endpoint (list applications) works correctly.

Usage Information

Request ID - Value of the requestId field if you are receiving a Graph API error response

329b85ba-dc7a-4712-b9b7-89e3907972c8

SDK Version - [SDK version you are using]

3.0.2

Node Version - [The version of Node you are using]

❯ node --version
v18.10.0

Browser Name - [The name of Browser that you are using for SDK]

Version - [The version of the browser you are using]

P.S.

Thank you good library. I'm happy to code with type informations.

nikithauc commented 2 years ago

@bigwheel Thank you for reporting this issue!

I appreciate your feedback of the library and the clear explanation of the issue :)

I apologize for the inconvenience caused! At this time, my recommendation is to continue using the library with the --no-experimental-fetch as it is experimental.

stefansundin commented 1 year ago

In Node.js 18.13.0 they removed the warning message about fetch being experimental, so I guess it is considered more stable now.

It would be great if it would be officially supported by this package. This package is the only reason we still have node-fetch in our code now.

sebastienlevert commented 1 year ago

This is good feedback. I don't know if this will land in this version (3.x) but I promise we'll be looking into it for our v4.x we are actively working on. I get the struggle and we really want to support native fetch.

seilorjunior commented 1 year ago

I got the same issue with the Node.js v18.15.0, do you have any plans to have this working?

Airdevelopments commented 1 year ago

For me adding the Accept-Language header manually to the request via .header('Accept-Language', 'en-US') seems to be a temporary workaround.

The full call for me would be:

const response = await this.client .api('/applications') .header('Accept-Language', 'en-US') .query({ '$filter': 'appId eq \'MY_APP_ID\'', }) .get();

rkdefra commented 4 months ago

Any update on this? The fetch polyfill check is insatiable! Native fetch has been part of node for a while now, but I can't find a work-around or a way to support current/LTS versions of node :disappointed: