phetsims / gas-properties

"Gas Properties" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 6 forks source link

CT: cannot set state while setting state #240

Closed pixelzoom closed 6 months ago

pixelzoom commented 6 months ago

I've only seen this once in CT, for gases-intro, 5/10/2024, 3:29:53 AM. Note that further down in the message, the error reported in https://github.com/phetsims/gas-properties/issues/239 ("bad holdConstant state") appears.

gases-intro : phet-io-state-fuzz : unbuilt
http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/phet-io-wrappers/state/?sim=gases-intro&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22gases-intro%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1715333393008%22%2C%22timestamp%22%3A1715333857007%7D
Assertion failed: cannot set state while setting state
Error: Assertion failed: cannot set state while setting state
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/assert/js/assert.js:28:13)
at assert (PhetioStateEngine.ts:232:14)
at setState (PhetioStateEngine.ts:280:9)
at setFullState (phetioEngine.ts:1158:33)
at apply (phetioCommandProcessor.ts:429:50)
at process (phetioCommandProcessor.ts:303:35)
at getReturn (phetioCommandProcessor.ts:311:17)
at Array.map
at map (phetioCommandProcessor.ts:301:29)
at processCommands (phetioCommandProcessor.ts:242:34)
[URL] http://128.138.93.172/continuous-testing/aqua/html/wrapper-test.html?url=..%2F..%2Fct-snapshots%2F1715333393008%2Fphet-io-wrappers%2Fstate%2F%3Fsim%3Dgases-intro%26locales%3D*%26phetioWrapperDebug%3Dtrue%26fuzz%26phetioDebug%3Dtrue&testInfo=%7B%22test%22%3A%5B%22gases-intro%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1715333393008%22%2C%22timestamp%22%3A1715333857007%7D
[NAVIGATED] http://128.138.93.172/continuous-testing/aqua/html/wrapper-test.html?url=..%2F..%2Fct-snapshots%2F1715333393008%2Fphet-io-wrappers%2Fstate%2F%3Fsim%3Dgases-intro%26locales%3D*%26phetioWrapperDebug%3Dtrue%26fuzz%26phetioDebug%3Dtrue&testInfo=%7B%22test%22%3A%5B%22gases-intro%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1715333393008%22%2C%22timestamp%22%3A1715333857007%7D
[ATTACHED]
[NAVIGATED] about:blank
[NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/phet-io-wrappers/state/?sim=gases-intro&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22gases-intro%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1715333393008%22%2C%22timestamp%22%3A1715333857007%7D
[ATTACHED]
[NAVIGATED] about:blank
[ATTACHED]
[NAVIGATED] about:blank
[CONSOLE] enabling assert
[NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/gases-intro/gases-intro_en.html?brand=phet-io&ea&postMessageOnError&sim=gases-intro&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22gases-intro%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1715333393008%22%2C%22timestamp%22%3A1715333857007%7D&frameTitle=source
[NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/gases-intro/gases-intro_en.html?brand=phet-io&ea&postMessageOnError&sim=gases-intro&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22gases-intro%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1715333393008%22%2C%22timestamp%22%3A1715333857007%7D&frameTitle=destination
[CONSOLE] enabling assert
[CONSOLE] enabling assert
[CONSOLE] continuous-test-wrapper-load
[CONSOLE] continuous-test-wrapper-load
[CONSOLE] Assertion failed: bad holdConstant state: temperature with numberOfParticles=0
[PAGE ERROR] Error: Error: Assertion failed: bad holdConstant state: temperature with numberOfParticles=0
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/assert/js/assert.js:28:13)
at IdealGasLawModel.assert.holdConstantProperty.link.phetioDependencies (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/gas-properties/js/common/model/IdealGasLawModel.js:191:17)
at TinyProperty.notifyLoop (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/TinyEmitter.js:176:7)
at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/TinyEmitter.js:154:18)
at StringUnionProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/ReadOnlyProperty.js:264:23)
at PhaseCallback.listener (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/ReadOnlyProperty.js:309:47)
at PropertyStateHandler.attemptToApplyPhases (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/PropertyStateHandler.js:224:41)
at PropertyStateHandler.undeferAndNotifyProperties (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/PropertyStateHandler.js:153:12)
at http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/PropertyStateHandler.js:57:12
at TinyEmitter.notifyLoop (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/TinyEmitter.js:176:7)
[PAGE ERROR] Error: Error: Assertion failed: bad holdConstant state: temperature with numberOfParticles=0
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/assert/js/assert.js:28:13)
at IdealGasLawModel.assert.holdConstantProperty.link.phetioDependencies (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/gas-properties/js/common/model/IdealGasLawModel.js:191:17)
at TinyProperty.notifyLoop (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/TinyEmitter.js:176:7)
at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/TinyEmitter.js:154:18)
at StringUnionProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/ReadOnlyProperty.js:264:23)
at PhaseCallback.listener (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/ReadOnlyProperty.js:309:47)
at PropertyStateHandler.attemptToApplyPhases (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/PropertyStateHandler.js:224:41)
at PropertyStateHandler.undeferAndNotifyProperties (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/PropertyStateHandler.js:153:12)
at http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/PropertyStateHandler.js:57:12
at TinyEmitter.notifyLoop (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/axon/js/TinyEmitter.js:176:7)
[CONSOLE] Assertion failed: cannot set state while setting state
[CONSOLE] continuous-test-wrapper-error
[CONSOLE] continuous-test-wrapper-error
[PAGE ERROR] Error: Error: Assertion failed: cannot set state while setting state
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/assert/js/assert.js:28:13)
at PhetioStateEngine.setState (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/PhetioStateEngine.js:218:15)
at PhetioStateEngine.setFullState (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/PhetioStateEngine.js:261:10)
at PhetioEngine.implementation (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/phetioEngine.js:989:34)
at PhetioCommandProcessor.process (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/phetioCommandProcessor.js:312:51)
at getReturn (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/phetioCommandProcessor.js:181:29)
at http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/phetioCommandProcessor.js:189:18
at Array.map (<anonymous>)
at PhetioCommandProcessor.processCommands (http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/phetioCommandProcessor.js:178:30)
at http://128.138.93.172/continuous-testing/ct-snapshots/1715333393008/chipper/dist/js/phet-io/js/phetioCommandProcessor.js:125:35
[CONSOLE] continuous-test-wrapper-error
[CONSOLE] continuous-test-wrapper-error

id: "Bayes Node Puppeteer"
Snapshot from 5/10/2024, 3:29:53 AM
pixelzoom commented 6 months ago

@zepumph @samreid Can you provide insight into what "cannot set state while setting state" means, and what might typically cause this error?

This assertion is in PhetioStateEngine.ts setState, which is the only place in the PhET-iO code base where I find isSettingStateProperty.value = true. So I'm guessing that setState is somehow being called recursively.

  /**
   * Sets the state from an object that represents the state.
   *
   * @param state - a json object (not a string) that specifies the values to set
   * @param scopeTandem - returns if the given Tandem is in the scope of a (partial) state
   */
  public setState( state: FullPhetioState, scopeTandem: Tandem ): void {
    assert && assert( !this.isSettingStateProperty.value, 'cannot set state while setting state' );
pixelzoom commented 6 months ago

I no longer see this in CT after resolving #239. But I'd still like to understand the nature of "cannot set state while setting state" errors.

samreid commented 6 months ago

My hypothesis/guess is that #239 assertion failed during a set state command, so the set state command did not exit cleanly. The wrapper did not get the message, and tried to set the state again. The sim, thinking it was still in the middle of a set state call, asserted out. Let's check in with @zepumph

zepumph commented 6 months ago

@samreid is exactly right. There is a race condition between kicking off a new setState call and throwing the error from the sim. I'm happy to add in a crash-handler to prevent this, but mostly all that matters is what caused the problem from the sim side (bad holdConstant). Let's see if gas-properties throws an error like this again (moot if there isn't an underlying error to be triggered in gas-properties).

pixelzoom commented 6 months ago

Thanks for the clarification. It sounds like addressing https://github.com/phetsims/gas-properties/issues/239 made this go away. So closing.