Fix inngest.send. Add Event Builder

[djfarrelly]

Summary

Fix and improve how events are sent using the Inngest client's send method.

Changes:

• New exceptions to be thrown to handle specific types of Inngest Event API and event sending errors
• InngestEventBuilder to expose a builder pattern to creating events
• Refactor Step.kt to merge and simplify data classes

Checklist

• Update documentation
• Added unit/integration tests

Related

• INN-3336

[linear]

INN-3336 Simplify inngest.send for sending events without require types

[darwin67]

name and data are mandatory so why not have an InngestEvent with name and data for the constructor, and then chain itself instead?

Example/Reference in Rust
https://github.com/inngest/inngest-rs/blob/main/inngest/src/event.rs#L21-L49

[darwin67]

Also since the InngestEvent is a data class with optional fields, honestly don't really need a builder?

[djfarrelly]

The general idea is that we'd have a couple different ways to do things, what you recommend and a builder pattern. It was recommended to have different options for users like this. The builder pattern might be preferred for users that want to set id or v but not have to include null arguments in a constructor.

[albertchae]

Following up here:

• Builder pattern is pretty established in Java because IIRC Java does not natively support optional parameters (unless that's changed in a recent version). If we use @JvmOverloads on a Kotlin constructor with optional params, it'll generate overloaded methods for us but not all the various permutations a user might want.
• So given that we should provide a builder at all, I think Darwin's point about putting the required parameters in the constructor is pretty good. That's what the Book example from this article on builders from the Effective Java book has

  // isbn and title are required
  public Builder(String isbn, String title) {
      this.isbn = isbn;
      this.title = title;
  }
  
  // genre is optional
  public Builder genre(Genre genre) {
      this.genre = genre;
      return this;
  }

  However looking at these builder examples from several popular Java libraries, none of them seem to put required parameters in the builder constructor. So I guess it can go either way
  • https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/util/UriComponentsBuilder.html#build()
  • https://hc.apache.org/httpcomponents-client-5.2.x/current/httpclient5/apidocs/org/apache/hc/client5/http/config/RequestConfig.Builder.html
  • https://grpc.github.io/grpc-java/javadoc/io/grpc/ServerBuilder.html
  • https://square.github.io/okhttp/3.x/okhttp/okhttp3/Request.Builder.html
• One point for Java interop if we use non nullable constructor arguments in Kotlin. They will get annotated with org.jetbrains.annotations.NotNull for Java and immediately throw java.lang.NullPointerException: Parameter specified as non-null is null if called with a null argument. Thanks @KiKoS0 for confirming this behavior.
• A minor question is whether we want to nest the builder as InngestEvent.Builder or keep it as InngestEventBuilder. As you can see from the examples of other builders I linked, this seems to be up to the preference of the library author.

[albertchae]

After propagating InngestEvent to enough places to get this PR to compile, this Event is no longer used. I don't know if the intent was to convert external InngestEvents to internal Events somewhere, but I wasn't sure where so I'm punting on that for this PR

[albertchae]

The PR originally changed this from Array to List which I think is a good change, but to reduce scope for this PR I am punting on it to a follow up

[albertchae]

Following up here:

• Builder pattern is pretty established in Java because IIRC Java does not natively support optional parameters (unless that's changed in a recent version). If we use @JvmOverloads on a Kotlin constructor with optional params, it'll generate overloaded methods for us but not all the various permutations a user might want.
• So given that we should provide a builder at all, I think Darwin's point about putting the required parameters in the constructor is pretty good. That's what the Book example from this article on builders from the Effective Java book has

  // isbn and title are required
  public Builder(String isbn, String title) {
      this.isbn = isbn;
      this.title = title;
  }
  
  // genre is optional
  public Builder genre(Genre genre) {
      this.genre = genre;
      return this;
  }

  However looking at these builder examples from several popular Java libraries, none of them seem to put required parameters in the builder constructor. So I guess it can go either way
  • https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/util/UriComponentsBuilder.html#build()
  • https://hc.apache.org/httpcomponents-client-5.2.x/current/httpclient5/apidocs/org/apache/hc/client5/http/config/RequestConfig.Builder.html
  • https://grpc.github.io/grpc-java/javadoc/io/grpc/ServerBuilder.html
  • https://square.github.io/okhttp/3.x/okhttp/okhttp3/Request.Builder.html
• One point for Java interop if we use non nullable constructor arguments in Kotlin. They will get annotated with org.jetbrains.annotations.NotNull for Java and immediately throw java.lang.NullPointerException: Parameter specified as non-null is null if called with a null argument. Thanks @KiKoS0 for confirming this behavior.
• A minor question is whether we want to nest the builder as InngestEvent.Builder or keep it as InngestEventBuilder. As you can see from the examples of other builders I linked, this seems to be up to the preference of the library author.