svix / svix-webhooks Goto Github PK

pyright just added a new check reportPrivateImportUsage which fails for our re-exported types in API

microsoft/pyright@9035f64

Create a readme file for all of the libraries

The readme should include:

• How to use the library - covered in the docs. Should only have simple examples here (see next point).
• Add simple examples for how to use the library
• How to contribute to the repo
• How to extend the repo in order to add your own custom logic on top of the library's validation (See)

Steps to reproduce:
In a console:

irb(main):022:0> svix = Svix::Client.new('sk_REDACTED')
=> #<Svix::Client:0x00007fc9bc511200 @application=#<Svix::ApplicationAPI:0x00007fc9bc510c10 @api=#<Svix::ApplicationApi:0x00007fc9bc510be8 @api_client=#<Svix::ApiClie...

irb(main):023:0> app = 'app_REDACTED'
=> "app_REDACTED"

irb(main):024:0> svix.endpoint.list(app)
Traceback (most recent call last):
        1: from (irb):24
NameError (undefined local variable or method `app_id' for #<Svix::EndpointAPI:0x00007fc9bc510b70>)

Caused by:
https://github.com/svix/svix-libs/blob/5e85d147bdf7d012e643cd85158d4cdaee414e90/ruby/lib/svix/endpoint_api.rb#L9-L11
Unlike the rest of the methods in EndpointAPI, the argument is named str but the code uses app_id.

It looks like there might have been ARM builds in the past, but they were removed.
Would it be possible to add ARM builds for Apple Silicon users?

Create initial ruby project structure & webhook verification functionality

Based on this comment: #302 (comment)

The main advantage is that we can know the correct server was setup even when not using wait-for in docker.

Thoughts:

• Behavior should probably be controlled with a flag (timeout) - disabled, infinite, whatever.
• Check if the libs themselves can handle it, or whether we need to.
• Can the "is active" check be a real connection, or should we just do a TCP connection check?

See #42 for more info.

We may get signatures such as:

v1,BADSIG v1,GOODSIG v2,BADSIG

And we need to make sure that all the libraries know how to handle this and verify correctly

This will make it less likely for people to get this wrong, though the experience will be slightly worse. Need to think about it.

See #42 for more info.

I noticed that when a request to Svix fails, the stacktrace doesn't contain a description of what went wrong exactly.

E.g. this is the stack trace you get when updating an endpoint fails:

...
2021-09-08T12:19:36.711851+00:00 app[web.1]: at java.base/java.lang.Thread.run(Thread.java:829)
2021-09-08T12:19:36.711851+00:00 app[web.1]: Caused by: com.svix.exceptions.ApiException:
2021-09-08T12:19:36.711851+00:00 app[web.1]: at com.svix.Utils.wrapInternalApiException(Utils.java:10)
2021-09-08T12:19:36.711851+00:00 app[web.1]: at com.svix.Endpoint.update(Endpoint.java:45)
2021-09-08T12:19:36.711852+00:00 app[web.1]: at util.SvixClient.updateEndpoint(SvixClient.java:57)
2021-09-08T12:19:36.711853+00:00 app[web.1]: ... 35 common frames omitted
2021-09-08T12:19:36.711853+00:00 app[web.1]: Caused by: com.svix.internal.ApiException:
2021-09-08T12:19:36.711853+00:00 app[web.1]: at com.svix.internal.ApiClient.handleResponse(ApiClient.java:1023)
2021-09-08T12:19:36.711854+00:00 app[web.1]: at com.svix.internal.ApiClient.execute(ApiClient.java:936)
2021-09-08T12:19:36.711854+00:00 app[web.1]: at com.svix.internal.api.EndpointApi.updateEndpointApiV1AppAppIdEndpointEndpointIdPutWithHttpInfo(EndpointApi.java:894)
2021-09-08T12:19:36.711854+00:00 app[web.1]: at com.svix.internal.api.EndpointApi.updateEndpointApiV1AppAppIdEndpointEndpointIdPut(EndpointApi.java:868)
2021-09-08T12:19:36.711854+00:00 app[web.1]: at com.svix.Endpoint.update(Endpoint.java:43)
2021-09-08T12:19:36.711855+00:00 app[web.1]: ... 36 common frames omitted

I assume there's something wrong with the request body, but I don't know what exactly.

Now I could write code to print out the response body inside the exception, but it would be nice if this was done out of the box.

Multiple options, not sure what's the right one:

• https://github.com/xd009642/tarpaulin
• https://github.com/mozilla/grcov
• llvm-cov as recommended in https://blog.rust-lang.org/inside-rust/2020/11/12/source-based-code-coverage.html

The current version relies on Chrono which is deprecated so rust-ksuid switched away from it. We should do too.

• Once we switch away from it, we can merge in #285.
• We also need to update SeaORM to a version that doesn't depend on Chrono (is there one already?) - It's merged: SeaQL/sea-orm#602

Superlinter was great on paper, but in practice, it's a bit slow, and the configuration is ugly.

Each language should just have its own linter file.

Bug Report

I am filing this as a bug report because this is very likely to show up sooner or later and will cause confusion ("where did this panic occur and why?!")

expect with a message is preferred over unwrap (if panics are intended and the state is unrecoverable) or replaced by proper error handling. Looking at all code here and below:

Version

At least 0.51.

The number of files we need to modify for a version bump is already getting out of hand ;)

Would be useful to figure out which library is linked to a specific bug.

Better fix of #273

Should keep it enabled by default, but we might want to allow custom tolerances or allow to disable the tolerance enforcement altogether.

Can be useful if you want to verify old payloads or if you have a bad clock.

Gradle instructions are inconsistent between github Readme and svix.com

Bug Report

Version

0.53.0 and 0.53.1

Description

Hey there,

My name is Tamir and I run security & quality assessment for Ruby libraries using Diffend.io.

While reviewing the changes for this lib, I noticed, that the vendor directory is being included.

Is this needed? Usually, it's added by mistake and causes:

a) confusion
b) problems when tracking changes
c) extensive library size

ref: LINK

Note that the package size grew from 62KB to 2.68MB due to the new vender added.

If the presence of vendor data is valid I would love the reasoning. Thanks!

For example, for ApplicationOut:

---

NOTE: This class is auto generated by OpenAPI Generator.
Ref: https://openapi-generator.tech

Do not edit the class manually.

Attributes:
  allowed_values (dict): The key is the tuple path to the attribute
      and the for var_name this is (var_name,). The value is a dict
with a capitalized key describing the allowed value and an allowed
value. These dicts store the allowed enum values.
  attribute_map (dict): The key is attribute name
      and the value is json key in definition.
  discriminator_value_class_map (dict): A dict to go from the discriminator
      variable value to the discriminator class name.
  validations (dict): The key is the tuple path to the attribute
      and the for var_name this is (var_name,). The value is a dict
that stores validations for max_length, min_length, max_items,
min_items, exclusive_maximum, inclusive_maximum, exclusive_minimum,
inclusive_minimum, and regex.
  additional_properties_type (tuple): A tuple of classes accepted
      as additional properties values.

I don't think we are caching at the moment. Potentially because the server is in a subdirectory so target is in server/target/, though we definitely should.

Generated Models are created with arguments in alphabetical order eg EventTypeIn("description", "name") I don't love this, we probably want to wrap these models or see if we can generate them with named arguments

We should setup typing so that mypy can detect them

Points: 0.5

Tasks:

• Readme
• Changelog (#5)
• Update package metadata (e.g. URL)
• Add license

Create initial php project structure & webhook verification functionality

There aren’t any properties on the Svix prototype, so spyOn operation on Svix.prototype.application isn't working.
This also means that jest.mock("svix") will also fail.

exposing the sivx instance outside to enble mock isn't desired approach.

@svix-liam

Also make it possible to configure your own instance of HTTPClient.

Expected Behavior

irb(main):001:0> require 'svix'
=> true
irb(main):002:0> svix = Svix::Client.new('api-key')
=> #<Svix::Client:0x00007fc5d825e4a0 @application=#<Svix::ApplicationAPI:0x00007fc5d825d848  ...
irb(main):003:0> svix.health.get
=> nil # I'd actually like endpoints like this and deletes that return 204 to return something more useful than nil, but this is the pattern I expect currently

Actual Behavior

irb(main):003:0> svix.health.get
Traceback (most recent call last):
        2: from (irb):3
        1: from (irb):4:in `rescue in irb_binding'
NoMethodError (undefined method `health' for #<Svix::Client:0x00007fc5dd831990>)

See the description in #1 for how it should be done.

run across all platforms (macOS, windows, linux)

Including deployment

As a first pass we should cover all the Webhook verification cases that go, php, & ruby cover. It would be good to look at the rspec file in ruby as IMO it's the easiest to read: https://github.com/svixhq/svix-libs/blob/main/ruby/spec/webhook_spec.rb

See comment on HN: https://news.ycombinator.com/item?id=30635160

Maybe it doesn't affect us, if we are using a rust implementation of DNS, but I'm not sure. Need to check what rust-tls does. Though maybe it's a good idea to switch.

format!() macro should be avoided in places where it is called repeatedly, e.g. inside loops like this https://github.com/svix/svix-webhooks/blob/main/server/svix-server/src/core/types.rs#L429. Even if that line is not inside a loop it will be called repeatedly on every property it serializes.

An alternative can be picked from this list https://github.com/hoodie/concatenation_benchmarks-rs and also check this discussion on Reddit https://www.reddit.com/r/rust/comments/t06hk7/string_concatenations_benchmarks_updated/.

Additional info: hoodie/concatenation_benchmarks-rs#11 (not merged yet at the time of reporting, but contains useful info on crates implementing fast concatenation).

Some of the macros benchmarked in that study have very good performance, but I would check their source and what they expand to before picking the winner. They all serve a slightly different purpose.

Take a look at regen_openapi.sh and all of the files in javascript and python to see how to generate from the openapi spec, configure the generator to not generate some of the files, and make it into a package.
You can also look at https://github.com/etesync/etebase-java for how to create a java package. Though it's an Android package, which is a bit different to normal Java packages.

Keep the same API structure as there is in Python and JS (e.g. dh.message.create()).

We should do it if very easy to do. Otherwise it can wait for now.

Feature Request

Run clippy in pedantic mode, -W clippy::pedantic.

Motivation

Right now clippy is run in standard mode, which is quite lenient and does not catch a lot of common nits. clippy::pedantic very rarely has false positives, but from experience it catches style mistakes that even relatively experienced rust devs make (looking at myself here....). Its suggestions will help very much with long term maintenance and enforce a stricter programming style. Especially when on-boarding new devs without Rust experience pedantic clippy acts as a teacher. :-)

This should be done ASAP while the code base is relatively small (at the time of writing, clippy raises 81 warnings). Adding this later on will be painful.

Proposal

Require -W clippy::pedantic in CI. This will require a bigger PR to implement all clippy suggestions so that CI passes.

Alternatives

• Punt it until later (not a good idea; will be painful);
• Ignore it (don't see a good reason to).