From 5cf8d25f4e0d5aef432b9032fc8bfee2dd17d042 Mon Sep 17 00:00:00 2001 From: Gregg Tavares Date: Mon, 13 Jan 2025 16:36:31 -0800 Subject: [PATCH] Fix error scope wrapper so it doesn't miss errors. --- src/error-scope-wrapper.ts | 17 +++-- test/tests/push-pop-error-tests.js | 104 ++++++++++++++++++++++++++--- 2 files changed, 105 insertions(+), 16 deletions(-) diff --git a/src/error-scope-wrapper.ts b/src/error-scope-wrapper.ts index 4d25a00..4282532 100644 --- a/src/error-scope-wrapper.ts +++ b/src/error-scope-wrapper.ts @@ -21,12 +21,14 @@ if (typeof GPUDevice !== 'undefined') { const promise = origPopErrorScope.call(device) .then(error => { - // If there was a currentErrorScope when we added pushed then remove our promise + // If there was a currentErrorScope and there was no error the remove our promise. if (currentErrorScope) { - const ndx = currentErrorScope.errors.indexOf(promise); - if (ndx) { - currentErrorScope.errors.splice(ndx, 1); - } + if (!error) { + const ndx = currentErrorScope.errors.indexOf(promise); + if (ndx) { + currentErrorScope.errors.splice(ndx, 1); + } + } } else { // there was no currentErrorScope so emit the error if (error) { @@ -102,7 +104,10 @@ if (typeof GPUDevice !== 'undefined') { throw new DOMException('popErrorScope called on empty error scope stack', 'OperationError'); } const errPromise = origFn.call(this); - return errorScope.errors.pop() ?? errPromise; + errorScope.errors.push(errPromise); + const errors = await Promise.all(errorScope.errors); + const error = errors.find(v => !!v); + return error ?? null; }; })(GPUDevice.prototype.popErrorScope); diff --git a/test/tests/push-pop-error-tests.js b/test/tests/push-pop-error-tests.js index 0178fe0..bdee862 100644 --- a/test/tests/push-pop-error-tests.js +++ b/test/tests/push-pop-error-tests.js @@ -1,17 +1,42 @@ import {describe} from '../mocha-support.js'; import { itWithDevice } from '../js/utils.js'; -import { assertFalsy, assertInstanceOf, assertTruthy } from '../assert.js'; +import { assertEqual, assertFalsy, assertInstanceOf, assertTruthy } from '../assert.js'; + +function causeError(device, filter) { + switch (filter) { + case 'validation': + device.createBuffer({ + size: 1024, + usage: 0xffff, // Invalid GPUBufferUsage + }); + break; + case 'out-of-memory': + device.createTexture({ + // One of the largest formats. With the base limits, the texture will be 256 GiB. + format: 'rgba32float', + usage: GPUTextureUsage.COPY_DST, + size: [ + device.limits.maxTextureDimension2D, + device.limits.maxTextureDimension2D, + device.limits.maxTextureArrayLayers, + ], + }); + break; + default: + throw Error('unreachable'); + } +} describe('test push/pop error scope', () => { - itWithDevice('test we get error (because they are being captured)', async (device) => { + itWithDevice('test we get error (because they are being captured)', async (device) => { device.pushErrorScope('validation'); device.createSampler({maxAnisotropy: 0}); const err = await device.popErrorScope(); assertInstanceOf(err, GPUValidationError); }); - itWithDevice('test we get errors nested', async (device) => { + itWithDevice('test we get errors nested', async (device) => { device.pushErrorScope('validation'); device.pushErrorScope('validation'); device.createSampler({maxAnisotropy: 0}); @@ -28,7 +53,7 @@ describe('test push/pop error scope', () => { assertFalsy(rootErr); }); - itWithDevice('test we get errors nested - more', async (device) => { + itWithDevice('test we get errors nested - more', async (device) => { device.pushErrorScope('validation'); device.pushErrorScope('validation'); device.pushErrorScope('validation'); @@ -47,7 +72,15 @@ describe('test push/pop error scope', () => { assertInstanceOf(rootErr, GPUValidationError); }); - itWithDevice('test we get uncaught errors', async (device) => { + itWithDevice('test we get errors with success after error', async (device) => { + device.pushErrorScope('validation'); + device.createSampler({maxAnisotropy: 0}); + device.queue.submit([]); + const err = await device.popErrorScope(); + assertInstanceOf(err, GPUValidationError); + }); + + itWithDevice('test we get uncaught errors', async (device) => { const promise = new Promise(resolve => { device.addEventListener('uncapturederror', (e) => { resolve(e.error); @@ -80,18 +113,69 @@ describe('test push/pop error scope', () => { assertInstanceOf(uncapturedError, GPUValidationError); }); - itWithDevice('test we get rejection for empty stack, not an exception', async (device) => { - let wasRejected = false; + describe('test scope filters work', () => { + + itWithDevice('out-of-memory', async (device) => { + let uncapturedError; + device.addEventListener('uncapturederror', e => { + uncapturedError = e.error; + }); + device.pushErrorScope('out-of-memory'); + device.pushErrorScope('validation'); + causeError(device, 'out-of-memory'); + const validationError = await device.popErrorScope(); + const outOfMemoryError = await device.popErrorScope(); + assertFalsy(validationError, 'no validation error'); + assertInstanceOf(outOfMemoryError, GPUOutOfMemoryError); + assertFalsy(uncapturedError, 'no uncaptured error'); + }); + + itWithDevice('validation', async (device) => { + let uncapturedError; + device.addEventListener('uncapturederror', e => { + uncapturedError = e.error; + }); + device.pushErrorScope('validation'); + device.pushErrorScope('out-of-memory'); + causeError(device, 'validation'); + const outOfMemoryError = await device.popErrorScope(); + const validationError = await device.popErrorScope(); + assertFalsy(outOfMemoryError, 'no out of memory error'); + assertInstanceOf(validationError, GPUValidationError); + assertFalsy(uncapturedError, 'no uncaptured error'); + }); + + itWithDevice('both', async (device) => { + let uncapturedError; + device.addEventListener('uncapturederror', e => { + uncapturedError = e.error; + }); + device.pushErrorScope('validation'); + device.pushErrorScope('out-of-memory'); + causeError(device, 'validation'); + causeError(device, 'out-of-memory'); + const outOfMemoryError = await device.popErrorScope(); + const validationError = await device.popErrorScope(); + assertInstanceOf(outOfMemoryError, GPUOutOfMemoryError); + assertInstanceOf(validationError, GPUValidationError); + assertFalsy(uncapturedError, 'no uncaptured error'); + }); + + }); + + itWithDevice('test we get rejection for empty stack, not an exception', async (device) => { + let rejectedError; try { await device.popErrorScope() .then(() => assertFalsy(true, 'should not succeed')) - .catch(() => { - wasRejected = true; + .catch((error) => { + rejectedError = error; }); } catch { assertFalsy(true, 'should not throw'); } - assertTruthy(wasRejected, 'was rejected'); + assertInstanceOf(rejectedError, DOMException); + assertEqual(rejectedError.name, 'OperationError'); }); });