Fixed #59

Author: calebsander

dist/types/recursive.js

@@ -158,27 +158,31 @@ class RecursiveType extends absolute_1.default {
      */
     writeValue(buffer, value) {
         this.isBuffer(buffer);
-        let writeValue = true;
         let bufferRecursiveLocations = recursiveLocations.get(buffer);
         if (bufferRecursiveLocations) {
             const targetLocation = bufferRecursiveLocations.get(value);
             if (targetLocation !== undefined) {
-                writeValue = false;
                 buffer.add(0x00);
                 const offset = buffer.length - targetLocation; //calculate offset to previous location
                 buffer.addAll(flexInt.makeValueBuffer(offset));
+                return;
             }
         }
         else {
             bufferRecursiveLocations = new Map;
             recursiveLocations.set(buffer, bufferRecursiveLocations);
         }
-        if (writeValue) {
-            buffer.add(0xFF);
-            //Keep track of the location before writing the data so that this location can be referenced by sub-values
-            bufferRecursiveLocations.set(value, buffer.length);
+        //Value has not yet been written to the buffer
+        buffer.add(0xFF);
+        //Keep track of the location before writing the data so that this location can be referenced by sub-values
+        bufferRecursiveLocations.set(value, buffer.length);
+        try {
             this.type.writeValue(buffer, value);
         }
+        catch (e) {
+            bufferRecursiveLocations.delete(value);
+            throw e;
+        }
     }
     consumeValue(buffer, offset) {
         const explicit = read_util_1.readBooleanByte(buffer, offset);

test/value-bytes/recursive.ts

@@ -4,6 +4,11 @@ import * as rec from '../../dist'
 import * as t from '../../dist'
 import {bufferFrom} from '../test-common'
 
+type Field = {a: Fields} | {b: boolean}
+interface Fields {
+	fields: Field[]
+}
+
 export = () => {
 	interface GraphNode {
 		links: Set<GraphNode>
@@ -221,6 +226,30 @@ export = () => {
 				0x00, 12
 	]))
 
+	{
+		const fieldsType = new t.RecursiveType<Fields>('fields')
+		rec.registerType({
+			type: new t.StructType({
+				fields: new t.ArrayType(
+					new t.ChoiceType<Field>([
+						new t.StructType({a: fieldsType}),
+						new t.StructType({b: new t.BooleanType})
+					])
+				)
+			}),
+			name: 'fields'
+		})
+		assert.throws(
+			() => fieldsType.valueBuffer({
+				fields: [
+					{a: null as any, b: true}, //this will try to write null with fieldsType
+					{a: null as any, b: 1 as any} //ensure null is not considered already written by fieldsType
+				]
+			}),
+			'No types matched: {a: null, b: 1}'
+		)
+	}
+
 	assert.throws(
 		() => selfReferenceType.readValue(bufferFrom([0xff, 0x00, 2])),
 		'Cannot find target at 0'

types/recursive.ts

@@ -165,26 +165,29 @@ export default class RecursiveType<E, READ_E extends E = E> extends AbsoluteType
 	 */
 	writeValue(buffer: AppendableBuffer, value: E) {
 		this.isBuffer(buffer)
-		let writeValue = true
 		let bufferRecursiveLocations = recursiveLocations.get(buffer)
 		if (bufferRecursiveLocations) {
 			const targetLocation = bufferRecursiveLocations.get(value)
 			if (targetLocation !== undefined) { //value has already been written to the buffer
-				writeValue = false
 				buffer.add(0x00)
 				const offset = buffer.length - targetLocation //calculate offset to previous location
 				buffer.addAll(flexInt.makeValueBuffer(offset))
+				return
 			}
 		}
 		else {
 			bufferRecursiveLocations = new Map
 			recursiveLocations.set(buffer, bufferRecursiveLocations)
 		}
-		if (writeValue) { //value has not yet been written to the buffer
-			buffer.add(0xFF)
-			//Keep track of the location before writing the data so that this location can be referenced by sub-values
-			bufferRecursiveLocations.set(value, buffer.length)
-			this.type.writeValue(buffer, value)
+
+		//Value has not yet been written to the buffer
+		buffer.add(0xFF)
+		//Keep track of the location before writing the data so that this location can be referenced by sub-values
+		bufferRecursiveLocations.set(value, buffer.length)
+		try { this.type.writeValue(buffer, value) }
+		catch (e) {
+			bufferRecursiveLocations.delete(value)
+			throw e
 		}
 	}
 	consumeValue(buffer: ArrayBuffer, offset: number): ReadResult<READ_E> {