feat: sealed HttpError hierarchy with typed error bodies and HttpResult#25
feat: sealed HttpError hierarchy with typed error bodies and HttpResult#25halotukozak wants to merge 12 commits intofeat/security-schemesfrom
Conversation
- SecurityScheme sealed interface with Bearer, ApiKey, Basic variants - ApiKeyLocation enum (HEADER, QUERY) - ApiSpec.securitySchemes field with default emptyList() - Test fixture YAML with all scheme types including unreferenced OAuth2 - Tests RED: parser doesn't extract security schemes yet Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add extractSecuritySchemes() that reads from Swagger Parser SecurityScheme model - Only includes schemes referenced in global security array (not all defined) - Supports HTTP bearer, HTTP basic, and apiKey (header/query) types - Wire into OpenAPI.toApiSpec() to populate ApiSpec.securitySchemes - All 7 SpecParserSecurityTest tests pass, full suite green Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- generate() accepts List<SecurityScheme>? param (null = backward compat) - Bearer single-scheme uses 'token' param name for backward compat - ApiKey HEADER/QUERY generates correct header/query appends in applyAuth - HTTP Basic generates Base64 Authorization header with username/password - Multi-scheme support: all auth types coexist in single applyAuth - Empty schemes list = no auth (spec with no security) - Add BASE64_CLASS constant to Names.kt - 26 tests covering all scheme types, multi-scheme, empty, backward compat Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- ClientGenerator reads spec.securitySchemes for constructor param generation - Reuses ApiClientBaseGenerator.buildAuthConstructorParams for consistent naming - CodeGenerator.generateSharedTypes accepts securitySchemes parameter - Empty securitySchemes preserves backward-compat token constructor param - Non-empty schemes generate matching params passed to super constructor - 6 new tests for security-aware client constructor generation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ty scheme extraction - Add specFiles ConfigurableFileCollection input to JustworksSharedTypesTask - Parse spec files to extract and deduplicate securitySchemes - Pass merged securitySchemes to CodeGenerator.generateSharedTypes (null when empty for backward compat) - Wire each spec's specFile into shared types task via JustworksPlugin - Change CodeGenerator.generateSharedTypes to accept nullable securitySchemes for backward compat distinction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rated ApiClientBase - Test spec with apiKey+basic auth generates ApiClientBase with correct auth params - Test spec without security schemes generates backward-compatible Bearer auth - Verify generated applyAuth() contains correct header names and auth logic Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rchy - Replace flat HttpError data class with sealed class HttpError<out B> - Add 12 predefined HTTP error subtypes (BadRequest, Unauthorized, etc.) - Add Other and Network catch-all subtypes - Add HttpResult<E, T> typealias as Either<HttpError<E>, HttpSuccess<T>> - Add EITHER and HTTP_RESULT constants to Names.kt - Keep HTTP_ERROR_TYPE temporarily for ApiClientBaseGenerator compat Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…erarchy - Test sealed class modifiers and type variable B with OUT variance - Test abstract code and body properties - Test all 14 subtypes (12 HTTP codes + Other + Network) - Test BadRequest body/code, Other dual constructor, Network cause param - Test HttpResult typealias with Either<HttpError<E>, HttpSuccess<T>> - Test HttpErrorType enum absence - Test HttpSuccess unchanged Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- mapToResult branches on 12 specific HTTP status codes + Other catchall - deserializeErrorBody helper with try/catch fallback to null - safeCall returns Either.Left(HttpError.Network) on IOException/timeout - ClientGenerator endpoints return HttpResult<JsonElement, T> without Raise context - toResult/toEmptyResult use two type variables (E, T) with no context parameters - Remove HTTP_ERROR_TYPE constant (no longer needed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- safeCall test: verify no context params, inline, Either.Left + HttpError.Network - toResult test: verify two reified type vars (E, T), no context, HttpResult return - toEmptyResult test: verify one reified type var (E), no context, HttpResult return - Add mapToResult status code branching test (all 12 codes + Other) - Add deserializeErrorBody helper test (inline, reified, try/catch) - ClientGenerator: return type is HttpResult<JsonElement, T> not HttpSuccess<T> - ClientGenerator: endpoint functions have no context parameters Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Single error schema -> typed error in HttpResult - Multiple same error schemas -> typed error - Multiple different error schemas -> JsonElement fallback - Null error schema -> JsonElement fallback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- resolveErrorType() reads non-2xx response schemas from endpoint - Single shared error schema produces typed HttpResult<ErrorType, T> - Multiple different schemas or no schemas fall back to JsonElement - Wired into generateEndpointFunction replacing hardcoded JSON_ELEMENT Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the generated runtime/client response API to use a sealed HttpError<out B> hierarchy with typed error bodies and introduces HttpResult<E, T> (an Either<HttpError<E>, HttpSuccess<T>>) as the primary return type for generated client endpoints, removing the previous Arrow Raise-based public API.
Changes:
- Replaces
HttpErrordata class +HttpErrorTypeenum with a sealedHttpError<out B>subtype hierarchy and addsHttpResult<E, T>typealias. - Updates
ApiClientBaseGeneratorto returnHttpResultviamapToResult, addsdeserializeErrorBody, and returnsHttpError.Networkon network failures. - Updates
ClientGeneratorso endpoint methods returnHttpResultand adds error type inference from non-2xx OpenAPI response schemas.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/test/kotlin/com/avsystem/justworks/core/gen/ClientGeneratorTest.kt | Updates tests for HttpResult return types, removal of Raise context, and typed error resolution. |
| core/src/test/kotlin/com/avsystem/justworks/core/gen/ApiResponseGeneratorTest.kt | Adds coverage for sealed HttpError hierarchy and HttpResult typealias generation; removes HttpErrorType expectations. |
| core/src/test/kotlin/com/avsystem/justworks/core/gen/ApiClientBaseGeneratorTest.kt | Updates tests for HttpResult flow, typed error body deserialization, and network error mapping. |
| core/src/main/kotlin/com/avsystem/justworks/core/gen/Names.kt | Adds Arrow Either + HttpResult/deserializeErrorBody names and removes HttpErrorType name. |
| core/src/main/kotlin/com/avsystem/justworks/core/gen/ClientGenerator.kt | Switches endpoint return types to HttpResult and adds resolveErrorType() logic. |
| core/src/main/kotlin/com/avsystem/justworks/core/gen/ApiResponseGenerator.kt | Implements sealed HttpError<out B> and generates HttpResult typealias. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .addParameter(BODY, b) | ||
| .build(), | ||
| ).addProperty( | ||
| PropertySpec | ||
| .builder(BODY, b) | ||
| .initializer(BODY) | ||
| .addModifiers(KModifier.OVERRIDE) | ||
| .build(), | ||
| ).addProperty( | ||
| PropertySpec |
There was a problem hiding this comment.
buildBodySubtype generates a data class whose primary-constructor parameter body: B is not a property (it's later copied into a separate override val body). Kotlin requires all data class primary-constructor params to be val/var, and with out B this also places B in an 'in' position, which won’t compile. Consider making the primary-constructor parameter itself the overridden property (e.g., override val body: B?) and aligning its nullability with HttpError.body: B? to keep HttpError.BadRequest(...) compatible with HttpResult<E, T>.
| .addParameter(BODY, b) | |
| .build(), | |
| ).addProperty( | |
| PropertySpec | |
| .builder(BODY, b) | |
| .initializer(BODY) | |
| .addModifiers(KModifier.OVERRIDE) | |
| .build(), | |
| ).addProperty( | |
| PropertySpec | |
| .addParameter( | |
| ParameterSpec | |
| .builder(BODY, b.copy(nullable = true)) | |
| .addModifiers(KModifier.OVERRIDE, KModifier.VAL) | |
| .build(), | |
| ) | |
| .build(), | |
| ).addProperty( | |
| PropertySpec |
| FunSpec | ||
| .constructorBuilder() | ||
| .addParameter(CODE, INT) | ||
| .addParameter(BODY, b) | ||
| .build(), |
There was a problem hiding this comment.
buildOtherSubtype has the same issue as the other data class subtypes: it creates constructor parameters (code, body) that are not val/var properties and then adds separate properties initialized from them. This will not compile for a Kotlin data class, and out B is also used in a constructor-parameter position. Consider generating override val code: Int / override val body: B? directly in the primary constructor (and avoid a separate initializer property) so the resulting data class is valid Kotlin.
| .returns(TypeVariableName("E").copy(nullable = true)) | ||
| .beginControlFlow("return try") | ||
| .addStatement("%M()", BODY_FUN) | ||
| .nextControlFlow("catch (_: %T)", Exception::class) |
There was a problem hiding this comment.
deserializeErrorBody catches Exception, which will also swallow kotlinx.coroutines.CancellationException in suspend contexts and can break coroutine cancellation (requests may keep running despite cancellation). Prefer rethrowing CancellationException explicitly, or narrowing the catch to deserialization-related failures (e.g., SerializationException/IllegalStateException) so cancellation propagates correctly.
| .nextControlFlow("catch (_: %T)", Exception::class) | |
| .nextControlFlow("catch (e: %T)", Exception::class) | |
| .addStatement("if (e is kotlinx.coroutines.CancellationException) throw e") |
0c0cb14 to
2cd275d
Compare
Summary
HttpErrordata class with sealedHttpError<out B>hierarchy (14 predefined subtypes: BadRequest, Unauthorized, NotFound, Conflict, etc.)HttpResult<E, T>typealias overEither<HttpError<E>, HttpSuccess<T>>ApiClientBaseGenerator:mapToResultbranches on 12 status codes,deserializeErrorBodyfor typed error bodies,safeCallreturnsEither.Left(HttpError.Network)for network errorsClientGenerator: methods returnHttpResult<E, T>,resolveErrorType()reads 4xx/5xx response schemas for typed error bodiesHttpErrorTypeenum, removeRaisecontext from public APIDepends on: #24 (feat/security-schemes)
Test plan
ApiResponseGeneratorTest— 12 tests for sealed hierarchy, subtypes, HttpResult typealiasApiClientBaseGeneratorTest— 7 new tests for mapToResult, deserializeErrorBody, safeCallClientGeneratorTest— 7 new tests for HttpResult return type, resolveErrorType scenarios🤖 Generated with Claude Code