Skip to content

Commit

Permalink
Schema: improve handleForbidden implementation with SyncScheduler (
Browse files Browse the repository at this point in the history
  • Loading branch information
gcanti authored Feb 1, 2025
1 parent 5426b2d commit 0275faa
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 54 deletions.
89 changes: 53 additions & 36 deletions packages/effect/src/ParseResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@
* @since 3.10.0
*/

import * as array_ from "./Array.js"
import * as cause_ from "./Cause.js"
import * as Arr from "./Array.js"
import * as Cause from "./Cause.js"
import { TaggedError } from "./Data.js"
import * as Effect from "./Effect.js"
import * as Either from "./Either.js"
import * as Exit from "./Exit.js"
import type { LazyArg } from "./Function.js"
import { dual } from "./Function.js"
import { globalValue } from "./GlobalValue.js"
import * as Inspectable from "./Inspectable.js"
import * as util_ from "./internal/schema/util.js"
import * as Option from "./Option.js"
import * as Predicate from "./Predicate.js"
import * as Runtime from "./Runtime.js"
import * as Scheduler from "./Scheduler.js"
import type * as Schema from "./Schema.js"
import * as AST from "./SchemaAST.js"
import type { Concurrency } from "./Types.js"
Expand All @@ -41,7 +42,7 @@ export type ParseIssue =
* @category model
* @since 3.10.0
*/
export type SingleOrNonEmpty<A> = A | array_.NonEmptyReadonlyArray<A>
export type SingleOrNonEmpty<A> = A | Arr.NonEmptyReadonlyArray<A>

/**
* @category model
Expand Down Expand Up @@ -912,7 +913,7 @@ const go = (ast: AST.AST, isDecoding: boolean): Parser => {
const concurrency = getConcurrency(ast)
const batching = getBatching(ast)
return (input: unknown, options) => {
if (!array_.isArray(input)) {
if (!Arr.isArray(input)) {
return Either.left(new Type(ast, input))
}
const allErrors = options?.errors === "all"
Expand Down Expand Up @@ -1010,7 +1011,7 @@ const go = (ast: AST.AST, isDecoding: boolean): Parser => {
// ---------------------------------------------
// handle rest element
// ---------------------------------------------
if (array_.isNonEmptyReadonlyArray(rest)) {
if (Arr.isNonEmptyReadonlyArray(rest)) {
const [head, ...tail] = rest
for (; i < len - tail.length; i++) {
const te = head(input[i], options)
Expand Down Expand Up @@ -1106,15 +1107,15 @@ const go = (ast: AST.AST, isDecoding: boolean): Parser => {
// compute result
// ---------------------------------------------
const computeResult = ({ es, output }: State) =>
array_.isNonEmptyArray(es) ?
Arr.isNonEmptyArray(es) ?
Either.left(new Composite(ast, input, sortByIndex(es), sortByIndex(output))) :
Either.right(sortByIndex(output))
if (queue && queue.length > 0) {
const cqueue = queue
return Effect.suspend(() => {
const state: State = {
es: array_.copy(es),
output: array_.copy(output)
es: Arr.copy(es),
output: Arr.copy(output)
}
return Effect.flatMap(
Effect.forEach(cqueue, (f) => f(state), { concurrency, batching, discard: true }),
Expand Down Expand Up @@ -1332,7 +1333,7 @@ const go = (ast: AST.AST, isDecoding: boolean): Parser => {
// compute result
// ---------------------------------------------
const computeResult = ({ es, output }: State) => {
if (array_.isNonEmptyArray(es)) {
if (Arr.isNonEmptyArray(es)) {
return Either.left(new Composite(ast, input, sortByIndex(es), output))
}
if (options?.propertyOrder === "original") {
Expand All @@ -1357,7 +1358,7 @@ const go = (ast: AST.AST, isDecoding: boolean): Parser => {
const cqueue = queue
return Effect.suspend(() => {
const state: State = {
es: array_.copy(es),
es: Arr.copy(es),
output: Object.assign({}, output)
}
return Effect.flatMap(
Expand Down Expand Up @@ -1481,7 +1482,7 @@ const go = (ast: AST.AST, isDecoding: boolean): Parser => {
// compute result
// ---------------------------------------------
const computeResult = (es: State["es"]) =>
array_.isNonEmptyArray(es) ?
Arr.isNonEmptyArray(es) ?
es.length === 1 && es[0][1]._tag === "Type" ?
Either.left(es[0][1]) :
Either.left(new Composite(ast, input, sortByIndex(es))) :
Expand All @@ -1491,7 +1492,7 @@ const go = (ast: AST.AST, isDecoding: boolean): Parser => {
if (queue && queue.length > 0) {
const cqueue = queue
return Effect.suspend(() => {
const state: State = { es: array_.copy(es) }
const state: State = { es: Arr.copy(es) }
return Effect.flatMap(
Effect.forEach(cqueue, (f) => f(state), { concurrency, batching, discard: true }),
() => {
Expand Down Expand Up @@ -1640,37 +1641,53 @@ const handleForbidden = <A, R>(
actual: unknown,
options: InternalOptions | undefined
): Effect.Effect<A, ParseIssue, R> => {
// If effects are allowed, return the original effect
if (options?.isEffectAllowed === true) {
return effect
}

// Convert the effect to an Either, if possible
const eu = eitherOrUndefined(effect)
// If the effect is already an Either, return it directly
if (eu) {
return eu
}
if (options?.isEffectAllowed === true) {
return effect
}
try {
return Effect.runSync(Effect.either(effect as Effect.Effect<A, ParseIssue>))
} catch (e) {
if (Runtime.isFiberFailure(e)) {
const cause = e[Runtime.FiberFailureCauseId]
if (cause_.isDieType(cause) && Runtime.isAsyncFiberException(cause.defect)) {
return Either.left(
new Forbidden(
ast,
actual,
"cannot be be resolved synchronously, this is caused by using runSync on an effect that performs async work"
)
)
}

// Otherwise, attempt to execute the effect synchronously
const scheduler = new Scheduler.SyncScheduler()
const fiber = Effect.runFork(effect as Effect.Effect<A, ParseIssue>, { scheduler })
scheduler.flush()
const exit = fiber.unsafePoll()

if (exit) {
if (Exit.isSuccess(exit)) {
// If the effect successfully resolves, wrap the value in a Right
return Either.right(exit.value)
}
return Either.left(new Forbidden(ast, actual, String(e)))
const cause = exit.cause
if (Cause.isFailType(cause)) {
// The effect executed synchronously but failed due to a ParseIssue
return Either.left(cause.error)
}
// The effect executed synchronously but failed due to a defect (e.g., a missing dependency)
return Either.left(new Forbidden(ast, actual, Cause.pretty(cause)))
}

// The effect could not be resolved synchronously, meaning it performs async work
return Either.left(
new Forbidden(
ast,
actual,
"cannot be be resolved synchronously, this is caused by using runSync on an effect that performs async work"
)
)
}

const compare = ([a]: [number, ...Array<unknown>], [b]: [number, ...Array<unknown>]) => a > b ? 1 : a < b ? -1 : 0

function sortByIndex<T>(
es: array_.NonEmptyArray<[number, T]>
): array_.NonEmptyArray<T>
es: Arr.NonEmptyArray<[number, T]>
): Arr.NonEmptyArray<T>
function sortByIndex<T>(es: Array<[number, T]>): Array<T>
function sortByIndex(es: Array<[number, any]>) {
return es.sort(compare).map((t) => t[1])
Expand Down Expand Up @@ -1809,7 +1826,7 @@ interface CurrentMessage {

const getCurrentMessage = (
issue: ParseIssue
): Effect.Effect<CurrentMessage, cause_.NoSuchElementException> =>
): Effect.Effect<CurrentMessage, Cause.NoSuchElementException> =>
getAnnotated(issue).pipe(
Option.flatMap(AST.getMessageAnnotation),
Effect.flatMap((annotation) => {
Expand Down Expand Up @@ -1841,7 +1858,7 @@ const isTransformation = createParseIssueGuard("Transformation")

const getMessage: (
issue: ParseIssue
) => Effect.Effect<string, cause_.NoSuchElementException> = (issue: ParseIssue) =>
) => Effect.Effect<string, Cause.NoSuchElementException> = (issue: ParseIssue) =>
getCurrentMessage(issue).pipe(
Effect.flatMap((currentMessage) => {
const useInnerMessage = !currentMessage.override && (
Expand Down Expand Up @@ -2017,7 +2034,7 @@ const formatArray = (
case "Composite":
return getArray(e, path, () =>
util_.isNonEmpty(e.issues)
? Effect.map(Effect.forEach(e.issues, (issue) => formatArray(issue, path)), array_.flatten)
? Effect.map(Effect.forEach(e.issues, (issue) => formatArray(issue, path)), Arr.flatten)
: formatArray(e.issues, path))
case "Refinement":
case "Transformation":
Expand Down
40 changes: 23 additions & 17 deletions packages/effect/test/Schema/Schema/decodeUnknownSync.test.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
import { describe, it } from "@effect/vitest"
import * as ParseResult from "effect/ParseResult"
import * as S from "effect/Schema"
import { Effect, ParseResult, Predicate, Schema as S } from "effect"
import * as Util from "effect/test/Schema/TestUtils"
import { assertIncludes, assertTrue } from "effect/test/util"
import { strictEqual, throws } from "effect/test/util"

const SyncEffectfulString = S.declare([], {
decode: () => (u, _, ast) =>
Predicate.isString(u) ? Effect.succeed(u) : Effect.fail(new ParseResult.Type(ast, u, "not a string")),
encode: () => (u, _, ast) =>
Predicate.isString(u) ? Effect.succeed(u) : Effect.fail(new ParseResult.Type(ast, u, "not a string"))
}, { identifier: "SyncEffectfulString" })

describe("decodeUnknownSync", () => {
it("the returned error should be a ParseError", () => {
try {
S.decodeUnknownSync(S.String)(1)
} catch (e) {
assertTrue(ParseResult.isParseError(e))
}
it("should return a ParseError when the input is invalid", () => {
Util.assertions.parseError(() => S.decodeUnknownSync(S.String)(1), "Expected string, actual 1")
})

it("should decode synchronously even when the schema uses Effects", () => {
strictEqual(S.decodeUnknownSync(SyncEffectfulString)("a"), "a")
Util.assertions.parseError(() => {
S.decodeUnknownSync(SyncEffectfulString)(null)
}, "not a string")
})

it("should throw on async", () => {
it("should throw an error when the schema performs asynchronous work", () => {
Util.assertions.parseError(
() => S.decodeUnknownSync(Util.AsyncString)("a"),
`AsyncString
└─ cannot be be resolved synchronously, this is caused by using runSync on an effect that performs async work`
)
})

it("should throw on unexpected dependencies", () => {
try {
S.decodeUnknownSync(Util.DependencyString as any)("a")
throw new Error("unexpected success")
} catch (e: any) {
assertIncludes(e.message, "Service not found: Name")
}
it("should throw an error when required dependencies are missing", () => {
throws(() => S.decodeUnknownSync(Util.DependencyString as any)("a"), (err) => {
return err instanceof ParseResult.ParseError && err.message.includes("Service not found: Name")
})
})
})
2 changes: 1 addition & 1 deletion packages/effect/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const config: ViteUserConfig = {
test: {
coverage: {
reporter: ["html"],
include: ["src/**/*.ts"],
include: ["src/ParseResult.ts"],
provider: "v8"
}
}
Expand Down

0 comments on commit 0275faa

Please sign in to comment.