1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-02-09 00:18:00 +01:00
Commit Graph

13 Commits

Author SHA1 Message Date
Thomas Heartman
2765ae2c70
feat: unify error responses (#3607)
This PR implements the first version of a suggested unification (and
documentation) of the errors that we return from the API today.

The goal is for this to be the first step towards the error type defined
in this internal [linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type
'Define the new API error type').

## The state of things today

As things stand, we currently have no (or **very** little) documentation
of the errors that are returned from the API. We mention error codes,
but never what the errors may contain.

Second, there is no specified format for errors, so what they return is
arbitrary, and based on ... Who knows? As a result, we have multiple
different errors returned by the API depending on what operation you're
trying to do. What's more, with OpenAPI validation in the mix, it's
absolutely possible for you to get two completely different error
objects for operations to the same endpoint.

Third, the errors we do return are usually pretty vague and don't really
provide any real help to the user. "You don't have the right
permissions". Great. Well what permissions do I need? And how would I
know? "BadDataError". Sick. Why is it bad?

... You get it.

## What we want to achieve

The ultimate goal is for error messages to serve both humans and
machines. When the user provides bad data, we should tell them what
parts of the data are bad and what they can do to fix it. When they
don't have the right permissions, we should tell them what permissions
they need.

Additionally, it would be nice if we could provide an ID for each error
instance, so that you (or an admin) can look through the logs and locate
he incident.

## What's included in **this** PR?

This PR does not aim to implement everything above. It's not intended to
magically fix everything. Its goal is to implement the necessary
**breaking** changes, so that they can be included in v5. Changing error
messages is a slightly grayer area than changing APIs directly, but
changing the format is definitely something I'd consider breaking.

So this PR:

- defines a minimal version of the error type defined in the [API error
definition linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type).
- aims to catch all errors we return today and wrap them in the error
type
-   updates tests to match the new expectations.

An important point: because we are cutting v5 very soon and because work
for this wasn't started until last week, the code here isn't necessarily
very polished. But it doesn't need to be. The internals can be as messy
as we want, as long as the API surface is stable.

That said, I'm very open to feedback about design and code completeness,
etc, but this has intentionally been done quickly.

Please also see my inline comments on the changes for more specific
details.

### Proposed follow-ups

As mentioned, this is the first step to implementing the error type. The
public API error type only exposes `id`, `name`, and `message`. This is
barely any more than most of the previous messages, but they are now all
using the same format. Any additional properties, such as `suggestion`,
`help`, `documentationLink` etc can be added as features without
breaking the current format. This is an intentional limitation of this
PR.

Regarding additional properties: there are some error responses that
must contain extra properties. Some of these are documented in the types
of the new error constructor, but not all. This includes `path` and
`type` properties on 401 errors, `details` on validation errors, and
more.

Also, because it was put together quickly, I don't yet know exactly how
we (as developers) would **prefer** to use these new error messages
within the code, so the internal API (the new type, name, etc), is just
a suggestion. This can evolve naturally over time if (based on feedback
and experience) without changing the public API.

## Returning multiple errors

Most of the time when we return errors today, we only return a single
error (even if many things are wrong). AJV, the OpenAPI integration we
use does have a setting that allows it to return all errors in a request
instead of a single one. I suggest we turn that on, but that we do it in
a separate PR (because it updates a number of other snapshots).

When returning errors that point to `details`, the objects in the
`details` now contain a new `description` property. This "deprecates"
the `message` property. Due to our general deprecation policy, this
should be kept around for another full major and can be removed in v6.

```json
{
  "name": "BadDataError",
  "message": "Something went wrong. Check the `details` property for more information."
  "details": [{
    "message": "The .params property must be an object. You provided an array.",
    "description": "The .params property must be an object. You provided an array.",
  }]
}
```
2023-04-25 13:40:46 +00:00
andreas-unleash
edd12709ef
fix: remove proxy return all toggles functionality (#3331)
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
This PR removes the return all toggles functionality. Removes the flag
as well
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->

<!-- Does it close an issue? Multiple? -->
Closes #
[1-778](https://linear.app/unleash/issue/1-778/remove-proxyalltoggles-functionality)

<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->

### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->


## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->

---------

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
2023-03-16 14:00:18 +02:00
Gastón Fournier
37ce81a727
chore: re-use the same client schema for proxy (#3251)
## About the changes
client-metrics-schema is less strict than proxy-metrics-schema because
the former allows empty `instanceId` and also supports dates as
timestamps as well as date-formatted strings.

Using the same schema makes sense to reduce maintainability costs and
it's less error-prone if we need to modify the schema because underlying
the schema they both use the same code.

The reasoning is that proxy metrics should align with our client
metrics. Alternatively, we have new endpoints for edge metrics that will
aggregate and bucket by client.


![image](https://user-images.githubusercontent.com/455064/222738911-4c443e02-3072-4042-bfde-327da8dd46fe.png)


## Discussion points
Will we ever want to evolve proxy-metrics differently than
client-metrics? I'm under the assumption that the answer is no
2023-03-03 16:36:23 +01:00
Gastón Fournier
9c214858c7
chore: backport changes from 4.20 (#3021)
## About the changes
Changes included in 4.20 release, mainly improving OpenAPI tags
2023-01-30 16:02:42 +01:00
Ivar Conradi Østhus
4a3d26065f
Fix/cors expose ETag (#2594)
This commit fixes two issues with the frontend API

1. fix: update cors max age to match chromium defaults
https://source.chromium.org/chromium/chromium/src/+/main:services/network/public/cpp/cors/preflight_result.cc;drc=49e7c0b4886cac1f3d09dc046bd528c9c811a0fa;l=31
2: fix: expose ETage for cross-origin requests
2022-12-05 10:04:35 +01:00
Fredrik Strand Oseberg
5d52216d53
fix: adds cors caching (#2522)
* This PR adds a configurable maxAge header to the CORS middleware. This
allows the preflight request to be cached so that we can reduce the
request load on our end for the frontend clients starting to utilise the
frontend api.
2022-11-24 16:14:47 +01:00
andreas-unleash
6e5b214475
implement proxy all endpoint (#2460)
Signed-off-by: andreas-unleash <andreas@getunleash.ai>

<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->

This PR implements the `all` endpoint of unleash-proxy, by adding an
experimental flag that can control the behaviour

## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->

<!-- Does it close an issue? Multiple? -->
Closes #

<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->

### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->


## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
2022-11-21 12:57:07 +02:00
Fredrik Strand Oseberg
b0626d46bc
fix: respect environment if set on context (#2206)
When using the frontend api (embedded proxy) we should allow the use to self-define the environment on the proxy.
2022-10-19 12:29:00 +02:00
olav
30dab4f367
fix: use correct proxy logger name (#2027) 2022-08-31 16:02:22 +02:00
Fredrik Strand Oseberg
85b45b9965
Feat/unleash flags embedded proxy (#1974)
* feat: use unleash flags for embedded proxy

* feat: add a separate flag for the proxy frontend

* fix: setup unleash in dev

* fix: check flagResolver on each request

* fix: remove unleash client setup

* refactor: update frontend routes snapshot

* refactor: make batchMetrics flag dynamic

* fix: always check dynamic CORS origins config

* fix: make conditionalMiddleware work with the OpenAPI schema generation

Co-authored-by: olav <mail@olav.io>
2022-08-26 15:16:29 +02:00
olav
42d64c8803
feat: add CORS instance settings (#1957)
* feat: add CORS instance settings

* refactor: disallow arbitrary asterisks in CORS origins
2022-08-26 09:09:48 +02:00
olav
0d293929f5
feat: add CORS support to the proxy endpoints (#1936)
* feat: add CORS support to the proxy endpoints

* refactor: remove unused development mode CORS support
2022-08-19 08:09:44 +02:00
olav
e8d542af0f
feat: embed proxy endpoints (#1926)
* refactor: remove unused API definition routes

* feat: add support for proxy keys

* feat: support listening for any event

* feat: embed proxy endpoints

* refactor: add an experimental flag for the embedded proxy
2022-08-16 15:33:33 +02:00