swagger-js icon indicating copy to clipboard operation
swagger-js copied to clipboard

Resolver error

Open jdegre opened this issue 4 years ago • 7 comments

Q&A (please complete the following information)

  • OS: Windows 10
  • Environment: Node.js v13.4.0
  • Method of installation: npm
  • Swagger-Client version: 3.10.8
  • Swagger/OpenAPI version: OpenAPI 3.0

Content & configuration

Swagger/OpenAPI definition:

The test consists of 3 cross-referenced YAML files: test.yaml (main), which references data types from 2 other YAML files (a.yaml and b.yaml). To perform the test, these 3 files should be placed in the same directory, or URL location.

test.yaml

openapi: 3.0.0
info:
  version: 1.0.0
  title: Title
paths:
  /foo:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Body'
      responses:
        '201':
          description: Created
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Body'
components:
  schemas:
    Body:
      type: object
      properties:
        p1:
          $ref: 'a.yaml#/components/schemas/A'
        p2:
          $ref: 'b.yaml#/components/schemas/B1'

a.yaml

openapi: 3.0.0
info:
  version: 1.0.0
  title: A
  description: A
paths: {}
components:
  schemas:
    A:
      type: object
      properties:
        p1:
          $ref: 'b.yaml#/components/schemas/B3'

b.yaml

openapi: 3.0.0
info:
  version: 1.0.0
  title: B
  description: B
paths: {}
components:
  schemas:
    B1:
      type: string
    B2:
      type: integer
    B3:
      type: object
      properties:
        p1:
          $ref: '#/components/schemas/B2'

Swagger-Client usage:

import SwaggerClient from 'swagger-client'
new SwaggerClient('https://<server>/test.yaml').then(swaggerClient => {
  console.log(swaggerClient.errors[0].message);
});

Describe the bug you're encountering

The output of the JS script above is:

Could not resolve reference: Could not resolve pointer: /components/schemas/B2 does not exist in document

To reproduce...

Steps to reproduce the behavior:

  1. Put the 3 files in a same URL location
  2. Execute the JS script above with node.js

Expected behavior

There should not be any error, since all the types can be resolved

Screenshots

Not needed

Additional context or thoughts

The same issue can be tested in the online Swagger Editor, by following the link: https://editor.swagger.io/?url=https://jdegre.github.io/test/test.yaml And, right after the test.yaml is loaded, in the right side of the window, expand the POST method, and you will see the same error:

Resolver error at paths./foo.post.responses.201.content.application/json.schema.properties.p1.properties.p1.properties.p1.$ref
Could not resolve reference: Could not resolve pointer: /components/schemas/B2 does not exist in document
Jump to line 18

jdegre avatar Jun 22 '20 17:06 jdegre

@jdegre thanks for reporting this. We'll look into it.

char0n avatar Jul 06 '20 10:07 char0n

These swagger-ui most likely related, with file reference issues: https://github.com/swagger-api/swagger-ui/issues/4678 https://github.com/swagger-api/swagger-ui/issues/4739 (also a separate circular reference on itself) https://github.com/swagger-api/swagger-ui/issues/5820 https://github.com/swagger-api/swagger-ui/issues/5705

Note that the $ref error 'Cannot read property '0' of undefined' is fixed in https://github.com/swagger-api/swagger-js/pull/1783. So there is still a file reference problem.

tim-lai avatar Oct 26 '20 22:10 tim-lai

any progress on this issue?

i have been browsing the source code and, with my very limited competence on JS, I suspect that the problem is on the "specmap" module.

note that, in the yaml files I posted in the OP, the main one (test.yaml) contains 2 identical references to "Body" data type. in one of them, the resolution works fine, but in the other, for some reason, the state of specmap context tree does not seem to be correct, and triggers the failed resolution.

sorry for bumping up this issue, but this problem is currently a showstopper for an entire community (3GPP - standardization of 5G mobile networks), so any feedback on this issue would be greatly appreciated!

jdegre avatar Nov 10 '21 16:11 jdegre

@jdegre,

I can reproduce, thanks for very good Steps To Reproduce. Yes the problem is probably in specmap. The pointer /components/schemas/B2 is being searched in a.yaml file, but should be searched in b.yaml file. baseDoc property of the specmap doesn't switch to b.yaml file properly.

I've isolated problem in this method: https://github.com/swagger-api/swagger-js/blob/3b5cc18fd7c738f3954609b0820459ef57e15c11/src/specmap/lib/context-tree.js#L24 Investigating further...

Additional interesting fact: even though the error is generated, the definition is eventually resolved correctly.

char0n avatar Dec 14 '21 16:12 char0n

Right, so after spending some time fully understanding the implementation, I've came into conclusion that you uncovered problem that I'm not sure we can fix in current architecture of the resolution mechanism. The current implementation of resolution has multiple problems that are very hard to address:

  • patches are being queued and then having the state they were generated against be modified before they are applied
  • using context tree and concurrency with promises creates only approximation of correct context tree
  • baseURLs are being computed from the context tree instead of being explicitly passed

All 3 points are main factors why you're getting errors while resolving documents documented in this issue. Eventually the documents are resolved correctly, but the process generates an error. This error is a false positive.

Instead of changing the entire algorithm, I'm thinking if we can produce a workaround to assert on generated errors and get rid of them if we establish that those errors are a false positive. I'll look into this ASAP. Meanwhile @jdegre can you explain why this issue is a showstopper? The only side effect which I can see is the generate error (false positive).

char0n avatar Dec 20 '21 08:12 char0n

Thanks a lot, @char0n, for taking the time to investigate the issue.

Instead of changing the entire algorithm, I'm thinking if we can produce a workaround to assert on generated errors and get rid of them if we establish that those errors are a false positive. I'll look into this ASAP.

That would be great, indeed. The main problem (for us) is that these "false positive" errors reported by the swagger-client resolver, are displayed by Swagger Editor/UI. Then, in complex and large APIs, it is almost impossible to tell if these errors are due to real problems in the API description, with incorrect $refs, or due to these "false positive" reports.

You may want to have a look at the issue in swagger-api/swagger-editor#1957. In that issue, @shockey commented that old versions of Swagger Editor (3.6.23 and earlier) dealt with the issue by wiping the error generated by swagger-client shortly after it was generated, so it was not actually displayed on the Swagger Editor/UI user interface.

Meanwhile @jdegre can you explain why this issue is a showstopper? The only side effect which I can see is the generate error (false positive).

You can have a look at an actual occurrence of the issue in real APIs, using the latest Swagger Editor version: https://editor.swagger.io/?url=https://raw.githubusercontent.com/jdegre/5GC_APIs/master/TS29518_Namf_Communication.yaml (it took me several hours to narrow down the behavior of that huge API, and convert it into the minimal example mentioned in the OP, in the "steps to reproduce).

If you expand the first PUT request on the right, you will see a huge number of errors (false positives) being reported (same happens if you expand all other HTTP methods). It was only after a crazy amount of time when we concluded that those were not real errors but false positives, and as commented above, it is impossible for us to use that (latest) Swagger Editor version because it is impossible to tell which errors are real resolver errors, or not. This have made us to have to stick to Swagger Editor version 3.6.23 (from 2019).

While this might seem like an annoyance, and not a real showstopper, we recently discovered that 3.6.23 had issues dealing with circular references (which were fixed at 3.6.24 onward), so that's the real showstopper: now, none of the versions is really usable for us anymore: 3.6.23 and earlier have Swagger Editor to block, and hang the UI, while resolving circular references; 3.6.24 and later have the problem of displaying absurd amount of "false positive" errors, making the UI almost unusable.

jdegre avatar Dec 20 '21 11:12 jdegre

Right, thanks for providing additional info. Eliminating false positives looks like a way to go.

Additional info:

Behavior changed:

Swagger-Editor https://github.com/swagger-api/swagger-editor/compare/v3.6.23...v3.6.24 The behavior change most likely came from SwaggerUI change in https://github.com/swagger-api/swagger-ui/compare/v3.20.9...v3.21.0

So before we'll try to come up with the remediation of false positives why these changes were made and how they relate to what we have today. Then we'll move forward.

char0n avatar Dec 20 '21 12:12 char0n