stoplightio / prism

Turn any OpenAPI2/3 and Postman Collection file into an API server with mocking, transformations and validations.
https://stoplight.io/open-source/prism
Apache License 2.0
4.3k stars 346 forks source link

Include payload as part of the error report #2326

Closed LasneF closed 1 year ago

LasneF commented 1 year ago

User Story Description

As a Tester, leveraging automation , CICD / Jenkins / newman / prism as a proxy of my backend

I want to my CICD to break when the contract validation is broken , and so prism to raise an error (currently ok with the --errors option) BUT i want as well that the error report contains also the payload that has raised the error

so I can fully understand why it has fails . This is very interesting for when error comes from backend, and for fine tuning

Acceptance Criteria

as part of the error , include the payload , that raised the error

can be by just setting the --errors flag , i can understand that there migh be security consideration so a good way would be to enable this feature adding another flag when launching prism in conjonction like --includePaylaod

chohmann commented 1 year ago

Hi @LasneF, we have a couple of follow up questions:

  1. What about your setup makes it impossible to record the payload sent as part of the test running and log that out of Prism responds with violations?
  2. Do you mean the full request and the full response (i.e including headers, etc.) to be logged?
  3. Could we provide more information in the violations already reported that would meet your needs?
LasneF commented 1 year ago

Here is my setup :

Jenkins trigger build build will launch

Use case is : Backend returns a 200 but with non valid payload according to the contract Given the fact we have an validation error, i want to diagnose what is wrong quickly .

I set the proxy to --error so that it breaks the newman test (that used to have the minimal check 200) and so will break the build

newman will then report the error with its html reporter , so that i can get the mistake but it will only give what prism would answer something like

request.body.colors.54 Request body property colors.54 must be equal to one of the allowed values: RED, GREEN, BLUE

the key here is i cannot grab from my logs the error but not the root cause , meaning what does the server returns, the return got catch by prism but not fowarded elsewhere.

currently prism returns the reason of the fail , but not the the cause. like i know it was not the actual the colors was of the item 54 is wrong, but don t know more

the idea whas here to return as part of the error a full report including the content of the payload that the server has produced. ideally it should contains the payload , and list of headers & value (i guess less usefull)

chohmann commented 1 year ago

@LasneF Thank you for explaining your setup!

Since Prism forwards the server's response your test runner should be able to log out the server's response that caused the validation error. Why do you want Prism to do that rather than Newman?

LasneF commented 1 year ago

Newman just cannot ! , newman will log what prism return , remind that there is for incoming message Newman ==> Prism ==> backend .

setting prism in proxy mode , for the return is making
Newman <== Prism <== Backend . Backend return its payload , that is analysed by prism and then transform to 'just the error', then foward it to Newman for sure newman trace the paylaod , but this is the payload that Prism has generated and so containing only the error , but not the root backend payload.

the topic would be either that prism foward the initial payload / header as part of the errors content

or to have traces in prism with the content the backend sent to it. I have tried to increase the trace level but unfortunately the content payload is not readable as sent as ascii integer char in a field called data like below that is not a sustainable option to have understanding of what has been sent

So i would see 2 options 1 ) given an option prism foward the response server to the client in the error fields . Having a specific option make sense for privacy and performance reason 2) and / or change the logger content in trace mode so that it display not only data as ascii integer but content

chohmann commented 1 year ago

@LasneF we believe there is a 3rd option that you can do today with no code changes required:

  1. run prism in proxy mode WITHOUT the --errors option (or --errors false)
  2. from Newman, right before making the request, log the request payload
  3. prism will return the actual server's response payload, because --errors option is false
  4. from Newman, receive the response payload and log as desired
  5. you can determine if there is a violation by checking if there's violations reported in the sl-violations header. It will look like this:
    [{"location":["request","body"],"severity":"Error","code":"required","message":"must have required property 'email'"}]
  6. fail the test if there's violations in the sl-violations header.

Let us know if this option works for you!

LasneF commented 1 year ago

This options would work i agree . The only issue i see is that it leaks the usage of prism inside the collection

the collection is shared to client, the concept was to include in the collection basics checks like 200 tests

and the CICD is here to validate that what we ship is 'workink' . Making prism in error mode was a nice way to spot that backend is not aligned ; and make the test failing . Here is would mean include the prism test in the newman , that would a bit leak that we leverage a great tool :) , it would be also systematic to include this for all postman collection , and a bit mess up the logs in postman tool (as postman already display the server logs)

so yes your solution make the deal , but let me hard time , and not super convient. To me it is something may be to put in your backlog . At leas here you have Customer requirement .

On a side note , the logs level trace of the server payload is not readable at all , this is also something that could be fixed and help a bit (log are trace with an array of integer :(

chohmann commented 1 year ago

@LasneF thank you for confirming our option works for you use case! Because that solves your use case, and we don't believe logging out entire payloads is something Prism should do, we're going to close this ticket.