Skip to content

Commit

Permalink
Propagate errors of promisified methods (#2567)
Browse files Browse the repository at this point in the history 
* types: allow desc for all objects

- closes #2558

* fix adapter with typing of common.desc

* fix type test

* add failing test

* now add implementation covering the error
  • Loading branch information
@foxriver76
foxriver76 authored Dec 23, 2023
1 parent 831bf3d commit 13fc9d3
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 35 deletions.
16 changes: 14 additions & 2 deletions packages/adapter/src/lib/adapter/adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ export interface AdapterClass {
/** Only emitted for compact instances */
on(event: 'exit', listener: (exitCode: number, reason: string) => Promise<void> | void): this;
on(event: 'log', listener: (info: any) => Promise<void> | void): this;
/** Extend an object and create it if it might not exist */
/**
* Extend an object and create it if it might not exist
* @deprecated use `adapter.extendObject` without callback instead
*/
extendObjectAsync(
id: string,
objPart: ioBroker.PartialObject,
Expand Down Expand Up @@ -416,17 +419,20 @@ export interface AdapterClass {

/**
* Writes a value into the states DB.
* @deprecated use `adapter.setState` without callback instead
*/
setStateAsync(
id: string,
state: ioBroker.State | ioBroker.StateValue | ioBroker.SettableState,
ack?: boolean
): ioBroker.SetStatePromise;
/** @deprecated use `adapter.setState` without callback instead */
setStateAsync(
id: string,
state: ioBroker.State | ioBroker.StateValue | ioBroker.SettableState,
options?: unknown
): ioBroker.SetStatePromise;
/** @deprecated use `adapter.setState` without callback instead */
setStateAsync(
id: string,
state: ioBroker.State | ioBroker.StateValue | ioBroker.SettableState,
Expand Down Expand Up @@ -2986,7 +2992,13 @@ export class AdapterClass extends EventEmitter {
}

// public signatures
extendObject(id: string, objPart: ioBroker.PartialObject): ioBroker.SetObjectPromise;
extendObject(id: string, objPart: ioBroker.PartialObject, callback?: ioBroker.SetObjectCallback): void;
extendObject(
id: string,
objPart: ioBroker.PartialObject,
options: ioBroker.ExtendObjectOptions
): ioBroker.SetObjectPromise;
extendObject(
id: string,
objPart: ioBroker.PartialObject,
Expand Down Expand Up @@ -3052,7 +3064,7 @@ export class AdapterClass extends EventEmitter {
* }
* ```
*/
extendObject(id: unknown, obj: unknown, options: unknown, callback?: unknown): any {
extendObject(id: unknown, obj: unknown, options?: unknown, callback?: unknown): Promise<any> | void {
if (typeof options === 'function') {
callback = options;
options = null;
Expand Down
68 changes: 37 additions & 31 deletions packages/common/src/lib/common/tools.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2318,42 +2318,48 @@ export function promisify(
const args = sliceArgs(arguments);
// @ts-expect-error we cannot know the type of `this`
context = context || this;
return new Promise<void | Record<string, any> | any[]>((resolve, reject) => {
fn.apply(
context,
args.concat([
function (error: string | Error, result: any) {
if (error) {
return reject(error instanceof Error ? error : new Error(error));
} else {
// decide on how we want to return the callback arguments
switch (arguments.length) {
case 1: // only an error was given
return resolve(); // Promise<void>
case 2: // a single value (result) was returned
return resolve(result);
default: {
// multiple values should be returned
let ret: Record<string, any> | any[];
// eslint-disable-next-line prefer-rest-params
const extraArgs = sliceArgs(arguments, 1);
if (returnArgNames && returnArgNames.length === extraArgs.length) {
// we can build an object
ret = {};
for (let i = 0; i < returnArgNames.length; i++) {
ret[returnArgNames[i]] = extraArgs[i];
// eslint-disable-next-line no-async-promise-executor
return new Promise<void | Record<string, any> | any[]>(async (resolve, reject) => {
try {
// await this to allow streamlining errors not passed via callback by async methods
await fn.apply(
context,
args.concat([
function (error: string | Error, result: any) {
if (error) {
return reject(error instanceof Error ? error : new Error(error));
} else {
// decide on how we want to return the callback arguments
switch (arguments.length) {
case 1: // only an error was given
return resolve(); // Promise<void>
case 2: // a single value (result) was returned
return resolve(result);
default: {
// multiple values should be returned
let ret: Record<string, any> | any[];
// eslint-disable-next-line prefer-rest-params
const extraArgs = sliceArgs(arguments, 1);
if (returnArgNames && returnArgNames.length === extraArgs.length) {
// we can build an object
ret = {};
for (let i = 0; i < returnArgNames.length; i++) {
ret[returnArgNames[i]] = extraArgs[i];
}
} else {
// we return the raw array
ret = extraArgs;
}
} else {
// we return the raw array
ret = extraArgs;
return resolve(ret);
}
return resolve(ret);
}
}
}
}
])
);
])
);
} catch (e) {
reject(e);
}
});
};
}
Expand Down
22 changes: 20 additions & 2 deletions packages/controller/test/lib/testObjectsFunctions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1359,7 +1359,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
return Promise.resolve();
});

// should use def as default state value on extendObject when obj non existing
// should use def as default state value on extendObject when obj non-existing
it(testName + 'Check extendObject state with def', async function () {
this.timeout(3_000);
let obj = await context.adapter.extendObjectAsync('testDefaultValExtend', {
Expand Down Expand Up @@ -1405,7 +1405,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
expect(state!.ack).to.equal(true);
});

// should use def as default state value on extendForeignObject when obj non existing
// should use def as default state value on extendForeignObject when obj non-existing
it(testName + 'Check extendForeignObject state with def', async () => {
let obj = await context.adapter.extendForeignObjectAsync('foreign.0.testDefaultValExtend', {
type: 'state',
Expand Down Expand Up @@ -1610,6 +1610,24 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
expect(objGet!.common.name).to.be.equal('CHANGED');
});

// test that real errors of methods promisified via tools.promisify are propagated, can be adapted to a more generic test
it(testName + 'Check that crashes of promisified methods are propagated', function () {
return expect(
context.adapter.extendObjectAsync('testDefaultValExtend', {
type: 'state',
common: {
type: 'string',
def: 'Run Forrest, Run!'
},
// @ts-expect-error force crash
native: -3
})
).to.be.eventually.rejectedWith(
`Cannot use 'in' operator to search for 'repositories' in -3`,
'Should have thrown'
);
});

it(testName + 'Should check object existence', async () => {
const id = 'objectExistenceCheckAdapter';
// object should not exist
Expand Down

0 comments on commit 13fc9d3

Please sign in to comment.