This PR fixes the bug report referenced later in this text. In short, there were two things:
The OpenAPI spec said the /proxy POST endpoint had a toggleNames property, but it's actually just toggles
If you POST invalid (according to the spec) JSON to the /proxy endpoint, then you get a 404.
These two issues have been addressed as follows:
The OpenAPI spec has been changed to list the property as toggles.
POSTing invalid data (that can't be parsed) now results in a 400. Posting valid json that doesn't conform to what we expect yields a 200. The decision for the latter is that there's been tests for this for a long time, so it's the expected behavior.
Root cause
The root cause of the 404 bug was that the OpenAPI validator required toggleNames. Further, the error handler that takes care of errors from the OpenAPI validator didn't catch other errors (such as syntax errors in the request payload). This has now been rectified.
Changes
The primary required changes are the ones in src/openapi/spec/lookup-toggles-schema.ts (adjusting the property name). There's also relevant updates to the tests in src/test/unleash-proxy.test.ts that now test more comprehensively for unexpected or malformed payloads.
Further, I have added 400 Bad Request as another standard response as it is shared by the POST endpoints and in doing so, also updated the common responses file (src/openapi/common-responses.ts) to be more in line with what we have in unleash/unleash and made it more type safe.
Bonus bug fix
There was a bug that is not currently caught by any of our tests: if you POST to /proxy asking for toggles that don't exist, you would get a 500 error, as the proxy tries to read the .impressionData property of undefined in src/client.ts. I've rectified this and manually confirmed that it works. However, because we use a mock client for testing, the tests don't surface this issue at all.
I would love some input on how to address this. If the only way is to set up e2e tests for it, then that may be worth it.
Bug report
The OAS specification here is wrong. It looks like the endpoint actually takes toggles and not toggleNames in the request body.
Also, when posting toggles instead of toggleNames the proxy returns an HTML error that says you can't post to this endpoint:
This PR fixes the bug report referenced later in this text. In short, there were two things:
toggleNamesproperty, but it's actually justtogglesThese two issues have been addressed as follows:
toggles.Root cause
The root cause of the 404 bug was that the OpenAPI validator required
toggleNames. Further, the error handler that takes care of errors from the OpenAPI validator didn't catch other errors (such as syntax errors in the request payload). This has now been rectified.Changes
The primary required changes are the ones in
src/openapi/spec/lookup-toggles-schema.ts(adjusting the property name). There's also relevant updates to the tests insrc/test/unleash-proxy.test.tsthat now test more comprehensively for unexpected or malformed payloads.Further, I have added 400 Bad Request as another standard response as it is shared by the POST endpoints and in doing so, also updated the common responses file (
src/openapi/common-responses.ts) to be more in line with what we have in unleash/unleash and made it more type safe.Bonus bug fix
There was a bug that is not currently caught by any of our tests: if you POST to /proxy asking for toggles that don't exist, you would get a 500 error, as the proxy tries to read the
.impressionDataproperty of undefined insrc/client.ts. I've rectified this and manually confirmed that it works. However, because we use a mock client for testing, the tests don't surface this issue at all.I would love some input on how to address this. If the only way is to set up e2e tests for it, then that may be worth it.
Bug report