effect icon indicating copy to clipboard operation
effect copied to clipboard

Creating an invalid Arbitrary hangs forever

Open wrporter opened this issue 7 months ago • 8 comments

What version of Effect is running?

3.14.11

What steps can reproduce the bug?

Attempt to make an Arbitrary with a schema that provides an invalid value for the filters of a property.

import { Arbitrary, Effect, FastCheck, Schema } from "effect"

const invalidSchema = Schema.Positive.annotations({
  arbitrary: () => (fc: typeof FastCheck) => fc.constant(null).map(() => -1)
})

const arb = Arbitrary.make(invalidSchema)
const sample = FastCheck.sample(arb, 3)

What is the expected behavior?

The Arbitrary gets created (previous behavior) or throws an error.

What do you see instead?

The process hangs forever on Arbitrary.make(invalidSchema).

Additional information

I've pinpointed this to [email protected] (playground) and it works as expected on [email protected] (playground)

wrporter avatar Apr 22 '25 12:04 wrporter

This is a bug fix rather than a new issue, previously it was producing invalid values.

gcanti avatar Apr 22 '25 12:04 gcanti

This is a bug fix rather than a new issue, previously it was producing invalid values.

Hey @gcanti thank you for the quick reply! I can understand if we don't want invalid values, but in that case, shouldn't we throw an error rather than hanging the process forever? This could be a major problem if it reaches a production environment.

wrporter avatar Apr 22 '25 12:04 wrporter

I think this is a responsibility that should be handled upstream by fast-check. This isn't the only situation where values might fail to be generated: for example, it can also happen without any overrides, just by setting overly strict filters. IIRC there are ways to stop the execution in such cases.

gcanti avatar Apr 22 '25 13:04 gcanti

I can't tell whether this is a problem with fast-check or with how Effect is consuming it.

Here's what I get with fast-check directly:

import fc from "fast-check"

const values = fc.sample(fc.integer().filter((n) => n > 0).map(() => -1), {
  numRuns: 3,
  interruptAfterTimeLimit: 1000,
  skipAllAfterTimeLimit: 1000,
  maxSkipsPerRun: 0,
  timeout: 1000,
  markInterruptAsFailure: true,
  verbose: true,
  endOnFailure: true
})

console.log(values)
// -> [ -1, -1, -1 ]

Whereas with Effect, it just hangs:

import { Arbitrary, FastCheck, Schema } from "effect"

const invalidSchema = Schema.Positive.annotations({
  arbitrary: () => (fc: typeof FastCheck) => fc.constant(null).map(() => -1)
})

const arb = Arbitrary.make(invalidSchema)

const sample = FastCheck.sample(arb, {
  numRuns: 1,
  interruptAfterTimeLimit: 100,
  skipAllAfterTimeLimit: 100,
  maxSkipsPerRun: 0,
  timeout: 100,
  markInterruptAsFailure: true,
  verbose: true,
  endOnFailure: true
})

Is this a comparable example?

wrporter avatar Apr 22 '25 14:04 wrporter

Is this a comparable example?

No, this demonstrates:

import { FastCheck as fc } from "effect"

// a filter that is not aligned with the base arbitrary
fc.sample(fc.integer({ max: 0 }).filter((n) => n > 0).map(() => -1), {
  numRuns: 3,
  interruptAfterTimeLimit: 1000,
  skipAllAfterTimeLimit: 1000,
  maxSkipsPerRun: 0,
  timeout: 1000,
  markInterruptAsFailure: true,
  verbose: true,
  endOnFailure: true
})

// hangs...

gcanti avatar Apr 22 '25 14:04 gcanti

Just wanted to highlight the fact that at the moment fast-check does not support filter operations never finding any valid values. It is expected to hang until it generates one or more valid values. On our side, we filled a feature-request to implement a way not to hang as we already do for fc.pre but so far we have not implemented it.

dubzzz avatar Apr 26 '25 21:04 dubzzz

@gcanti Depending on how blocking and expecting this case is, I can probably try to prioritize the support on my end. Anyway changing the behavior of filter will require me a new major as it could break existing clients that are really using the fact that we retry until we find something.

dubzzz avatar Apr 26 '25 21:04 dubzzz

@dubzzz thanks for chime in

I think fast-check's current behavior is fine, the important thing from our perspective is that we are not using it incorrectly.

Ideally (and maybe this would not even require a breaking change, but you are the best judge of that), it would be good to have an opt-in option to stop execution after a certain number of iterations, to avoid potential hangs.

gcanti avatar Apr 27 '25 07:04 gcanti