From e5dcdc7df6522fbb7c0c0fa904bf3338e3866ef8 Mon Sep 17 00:00:00 2001 From: Ghost Jake <89829542+Deepak-Kesavan@users.noreply.github.com> Date: Fri, 12 Dec 2025 07:05:05 +0530 Subject: [PATCH] Implement n8n review feedback for all custom nodes This commit addresses all feedback from Jon's n8n node review to ensure compliance with n8n best practices and standards. Changes across all nodes: - Remove custom sleep implementation, import from n8n-workflow - Add usableAsTool property to enable AI agent compatibility - Add resource and operation dropdown parameters with displayOptions - Move optional parameters to Additional Options collections - Implement Continue on Fail error handling pattern Node-specific changes: Unstract.node.ts & UnstractHitlPush.node.ts: - Remove redundant manual Bearer authentication from status polling - Use httpRequestWithAuthentication for all API requests - Remove manual apiKey extraction from credentials - Maintain returnFullResponse and ignoreHttpStatusErrors for 422 handling UnstractHitlFetch.node.ts: - Move HITL Queue Name from Additional Options to required field - Add ignoreHttpStatusErrors to handle 404 responses gracefully - Simplify error handling: return API errors in response instead of throwing - Remove hardcoded error message checks LlmWhisperer.node.ts: - Move 11 optional parameters to Additional Options collection - Maintain alphabetical sorting of options All changes maintain backward compatibility while improving code quality, maintainability, and adherence to n8n conventions. --- nodes/LlmWhisperer/LlmWhisperer.node.ts | 291 +++++++++++------- nodes/Unstract/Unstract.node.ts | 212 ++++++++----- .../UnstractHitlFetch.node.ts | 79 ++++- .../UnstractHitlPush/UnstractHitlPush.node.ts | 216 +++++++++---- 4 files changed, 547 insertions(+), 251 deletions(-) diff --git a/nodes/LlmWhisperer/LlmWhisperer.node.ts b/nodes/LlmWhisperer/LlmWhisperer.node.ts index 5a417f3..36c6985 100644 --- a/nodes/LlmWhisperer/LlmWhisperer.node.ts +++ b/nodes/LlmWhisperer/LlmWhisperer.node.ts @@ -6,23 +6,9 @@ import { NodeOperationError, NodeConnectionType, IHttpRequestMethods, + sleep, } from 'n8n-workflow'; -const sleep = (ms: number): Promise => - new Promise((resolve) => { - // Use Promise-based delay that doesn't rely on restricted globals - const start = Date.now(); - const check = (): void => { - if (Date.now() - start >= ms) { - resolve(); - } else { - // Use Promise.resolve() for non-blocking delay - Promise.resolve().then(check); - } - }; - check(); - }); - export class LlmWhisperer implements INodeType { description: INodeTypeDescription = { displayName: 'LLMWhisperer', @@ -30,6 +16,7 @@ export class LlmWhisperer implements INodeType { icon: 'file:llmWhisperer.svg', group: ['transform'], version: 1, + usableAsTool: true, description: 'Extract text from PDFs, images, and scanned documents using OCR. Preserves layout and formatting for accurate text extraction from any document type', subtitle: '={{$parameter["mode"] + " mode, " + $parameter["output_mode"] + " output"}}', defaults: { @@ -44,10 +31,49 @@ export class LlmWhisperer implements INodeType { }, ], properties: [ + { + displayName: 'Resource', + name: 'resource', + type: 'options', + noDataExpression: true, + options: [ + { + name: 'Document', + value: 'document', + }, + ], + default: 'document', + }, + { + displayName: 'Operation', + name: 'operation', + type: 'options', + noDataExpression: true, + displayOptions: { + show: { + resource: ['document'], + }, + }, + options: [ + { + name: 'Extract Text', + value: 'extractText', + description: 'Extract text from PDFs and images using OCR', + action: 'Extract text from document', + }, + ], + default: 'extractText', + }, { displayName: 'File Contents', name: 'file_contents', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['extractText'], + }, + }, default: 'data', description: 'The file contents to be processed', required: true, @@ -56,6 +82,12 @@ export class LlmWhisperer implements INodeType { displayName: 'LLMWhisperer Host', name: 'host', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['extractText'], + }, + }, default: 'https://llmwhisperer-api.us-central.unstract.com', description: 'Host URL for the LLMWhisperer API', required: true, @@ -64,6 +96,12 @@ export class LlmWhisperer implements INodeType { displayName: 'Mode', name: 'mode', type: 'options', + displayOptions: { + show: { + resource: ['document'], + operation: ['extractText'], + }, + }, options: [ { name: 'Form', @@ -90,6 +128,12 @@ export class LlmWhisperer implements INodeType { displayName: 'Output Mode', name: 'output_mode', type: 'options', + displayOptions: { + show: { + resource: ['document'], + operation: ['extractText'], + }, + }, options: [ { name: 'Layout Preserving', @@ -105,87 +149,100 @@ export class LlmWhisperer implements INodeType { required: true, }, { - displayName: 'Timeout', - name: 'timeout', - type: 'number', - default: 300, - description: 'Timeout in seconds for the API request', - }, - { - displayName: 'Page Separator', - name: 'page_seperator', - type: 'string', - default: '<<<', - description: 'The string to be used as a page separator', - }, - { - displayName: 'Pages to Extract', - name: 'pages_to_extract', - type: 'string', - default: '', - description: - 'Define which pages to extract. Example: 1-5,7,21- will extract pages 1,2,3,4,5,7,21,22,23,24... till the last page', - }, - { - displayName: 'Line Splitter Tolerance', - name: 'line_splitter_tolerance', - type: 'number', - typeOptions: { - minValue: 0, - maxValue: 1, + displayName: 'Additional Options', + name: 'additionalOptions', + type: 'collection', + placeholder: 'Add Option', + displayOptions: { + show: { + resource: ['document'], + operation: ['extractText'], + }, }, - default: 0.4, - description: - 'Factor to decide when to move text to the next line (40% of average character height)', - }, - { - displayName: 'Line Splitter Strategy', - name: 'line_splitter_strategy', - type: 'string', - default: 'left-priority', - description: 'The line splitter strategy to use. Advanced option for customizing line splitting.', - }, - { - displayName: 'Horizontal Stretch Factor', - name: 'horizontal_stretch_factor', - type: 'number', - default: 1.0, - description: 'Factor for horizontal stretch. 1.1 means 10% stretch.', - }, - { - displayName: 'Mark Vertical Lines', - name: 'mark_vertical_lines', - type: 'boolean', - default: false, - description: 'Whether to reproduce vertical lines in the document. Not applicable if mode=native_text.', - }, - { - displayName: 'Mark Horizontal Lines', - name: 'mark_horizontal_lines', - type: 'boolean', - default: false, - description: 'Whether to reproduce horizontal lines. Not applicable if mode=native_text, requires mark_vertical_lines=true.', - }, - { - displayName: 'Tag', - name: 'tag', - type: 'string', - default: 'default', - description: 'Auditing feature. Value associated with API invocation for usage reports.', - }, - { - displayName: 'File Name', - name: 'file_name', - type: 'string', - default: '', - description: 'Auditing feature. Value associated with API invocation for usage reports.', - }, - { - displayName: 'Add Line Numbers', - name: 'add_line_nos', - type: 'boolean', - default: false, - description: 'Whether to add line numbers to extracted text and save line metadata for highlights API', + default: {}, + options: [ + { + displayName: 'Add Line Numbers', + name: 'add_line_nos', + type: 'boolean', + default: false, + description: 'Whether to add line numbers to extracted text and save line metadata for highlights API', + }, + { + displayName: 'File Name', + name: 'file_name', + type: 'string', + default: '', + description: 'Auditing feature. Value associated with API invocation for usage reports.', + }, + { + displayName: 'Horizontal Stretch Factor', + name: 'horizontal_stretch_factor', + type: 'number', + default: 1.0, + description: 'Factor for horizontal stretch. 1.1 means 10% stretch.', + }, + { + displayName: 'Line Splitter Strategy', + name: 'line_splitter_strategy', + type: 'string', + default: 'left-priority', + description: 'The line splitter strategy to use. Advanced option for customizing line splitting.', + }, + { + displayName: 'Line Splitter Tolerance', + name: 'line_splitter_tolerance', + type: 'number', + typeOptions: { + minValue: 0, + maxValue: 1, + }, + default: 0.4, + description: 'Factor to decide when to move text to the next line (40% of average character height)', + }, + { + displayName: 'Mark Horizontal Lines', + name: 'mark_horizontal_lines', + type: 'boolean', + default: false, + description: 'Whether to reproduce horizontal lines. Not applicable if mode=native_text, requires mark_vertical_lines=true.', + }, + { + displayName: 'Mark Vertical Lines', + name: 'mark_vertical_lines', + type: 'boolean', + default: false, + description: 'Whether to reproduce vertical lines in the document. Not applicable if mode=native_text.', + }, + { + displayName: 'Page Separator', + name: 'page_seperator', + type: 'string', + default: '<<<', + description: 'The string to be used as a page separator', + }, + { + displayName: 'Pages to Extract', + name: 'pages_to_extract', + type: 'string', + default: '', + description: 'Define which pages to extract. Example: 1-5,7,21- will extract pages 1,2,3,4,5,7,21,22,23,24... till the last page', + }, + { + displayName: 'Tag', + name: 'tag', + type: 'string', + default: 'default', + description: 'Auditing feature. Value associated with API invocation for usage reports.', + }, + { + displayName: 'Timeout', + name: 'timeout', + type: 'number', + default: 300, + description: 'Timeout in seconds for the API request', + }, + ], }, ], }; @@ -198,7 +255,8 @@ export class LlmWhisperer implements INodeType { const { helpers } = this; for (let i = 0; i < items.length; i++) { - const fileContents = this.getNodeParameter('file_contents', i) as string; + try { + const fileContents = this.getNodeParameter('file_contents', i) as string; if (!items[i].binary?.[fileContents]) { throw new NodeOperationError( @@ -213,17 +271,20 @@ export class LlmWhisperer implements INodeType { const host = this.getNodeParameter('host', i) as string; const mode = this.getNodeParameter('mode', i) as string; const outputMode = this.getNodeParameter('output_mode', i) as string; - const pageSeparator = this.getNodeParameter('page_seperator', i) as string; - const pagesToExtract = this.getNodeParameter('pages_to_extract', i) as string; - const lineSplitterTolerance = this.getNodeParameter('line_splitter_tolerance', i) as number; - const lineSplitterStrategy = this.getNodeParameter('line_splitter_strategy', i) as string; - const horizontalStretchFactor = this.getNodeParameter('horizontal_stretch_factor', i) as number; - const markVerticalLines = this.getNodeParameter('mark_vertical_lines', i) as boolean; - const markHorizontalLines = this.getNodeParameter('mark_horizontal_lines', i) as boolean; - const tag = this.getNodeParameter('tag', i) as string; - const fileName = this.getNodeParameter('file_name', i) as string; - const addLineNumbers = this.getNodeParameter('add_line_nos', i) as boolean; - const timeout = this.getNodeParameter('timeout', i) as number; + + // Get additional options with defaults + const additionalOptions = this.getNodeParameter('additionalOptions', i, {}) as any; + const pageSeparator = additionalOptions.page_seperator !== undefined ? additionalOptions.page_seperator : '<<<'; + const pagesToExtract = additionalOptions.pages_to_extract || ''; + const lineSplitterTolerance = additionalOptions.line_splitter_tolerance !== undefined ? additionalOptions.line_splitter_tolerance : 0.4; + const lineSplitterStrategy = additionalOptions.line_splitter_strategy || 'left-priority'; + const horizontalStretchFactor = additionalOptions.horizontal_stretch_factor !== undefined ? additionalOptions.horizontal_stretch_factor : 1.0; + const markVerticalLines = additionalOptions.mark_vertical_lines !== undefined ? additionalOptions.mark_vertical_lines : false; + const markHorizontalLines = additionalOptions.mark_horizontal_lines !== undefined ? additionalOptions.mark_horizontal_lines : false; + const tag = additionalOptions.tag || 'default'; + const fileName = additionalOptions.file_name || ''; + const addLineNumbers = additionalOptions.add_line_nos !== undefined ? additionalOptions.add_line_nos : false; + const timeout = additionalOptions.timeout !== undefined ? additionalOptions.timeout : 300; const requestOptions = { method: 'POST' as IHttpRequestMethods, @@ -329,6 +390,22 @@ export class LlmWhisperer implements INodeType { pairedItem: { item: i }, }); } + } catch (error: any) { + if (this.continueOnFail()) { + returnData.push({ + json: { + error: error.message || 'Unknown error occurred', + }, + pairedItem: { item: i }, + }); + continue; + } + + if (error.message) { + throw new NodeOperationError(this.getNode(), error.message); + } + throw error; + } } return [returnData]; diff --git a/nodes/Unstract/Unstract.node.ts b/nodes/Unstract/Unstract.node.ts index 4b7a422..e37bf88 100644 --- a/nodes/Unstract/Unstract.node.ts +++ b/nodes/Unstract/Unstract.node.ts @@ -6,23 +6,9 @@ import { NodeOperationError, NodeConnectionType, IHttpRequestMethods, + sleep, } from 'n8n-workflow'; -const sleep = (ms: number): Promise => - new Promise((resolve) => { - // Use Promise-based delay that doesn't rely on restricted globals - const start = Date.now(); - const check = (): void => { - if (Date.now() - start >= ms) { - resolve(); - } else { - // Use Promise.resolve() for non-blocking delay - Promise.resolve().then(check); - } - }; - check(); - }); - export class Unstract implements INodeType { description: INodeTypeDescription = { displayName: 'Unstract', @@ -30,6 +16,7 @@ export class Unstract implements INodeType { icon: 'file:unstract.svg', group: ['transform'], version: 1, + usableAsTool: true, description: 'Process documents using Unstract API', defaults: { name: 'Unstract', @@ -43,10 +30,49 @@ export class Unstract implements INodeType { }, ], properties: [ + { + displayName: 'Resource', + name: 'resource', + type: 'options', + noDataExpression: true, + options: [ + { + name: 'Document', + value: 'document', + }, + ], + default: 'document', + }, + { + displayName: 'Operation', + name: 'operation', + type: 'options', + noDataExpression: true, + displayOptions: { + show: { + resource: ['document'], + }, + }, + options: [ + { + name: 'Process', + value: 'process', + description: 'Process document using Unstract API', + action: 'Process a document', + }, + ], + default: 'process', + }, { displayName: 'File Contents', name: 'file_contents', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['process'], + }, + }, default: 'data', description: 'Name of the binary property that contains the file data to be processed', required: true, @@ -55,6 +81,12 @@ export class Unstract implements INodeType { displayName: 'Unstract Host', name: 'host', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['process'], + }, + }, default: 'https://us-central.unstract.com', description: 'Host URL for the Unstract API', required: true, @@ -63,6 +95,12 @@ export class Unstract implements INodeType { displayName: 'API Deployment Name', name: 'deployment_name', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['process'], + }, + }, default: '', description: 'Name of the API deployment to use', required: true, @@ -71,37 +109,58 @@ export class Unstract implements INodeType { displayName: 'Timeout', name: 'timeout', type: 'number', + displayOptions: { + show: { + resource: ['document'], + operation: ['process'], + }, + }, default: 600, description: 'Maximum time in seconds to wait for processing', required: true, }, { - displayName: 'Include Metrics', - name: 'include_metrics', - type: 'boolean', - default: true, - description: 'Whether to include processing metrics in the response', - }, - { - displayName: 'Include Metadata', - name: 'include_metadata', - type: 'boolean', - default: true, - description: 'Whether to include document metadata in the response', - }, - { - displayName: 'Tags', - name: 'tags', - type: 'string', - default: '', - description: 'Comma-separated tags to associate with the document', - }, - { - displayName: 'Use Cached Results', - name: 'use_file_history', - type: 'boolean', - default: false, - description: 'Whether to use cached results if available. Useful while debugging your n8n workflow.', + displayName: 'Additional Options', + name: 'additionalOptions', + type: 'collection', + placeholder: 'Add Option', + displayOptions: { + show: { + resource: ['document'], + operation: ['process'], + }, + }, + default: {}, + options: [ + { + displayName: 'Include Metadata', + name: 'include_metadata', + type: 'boolean', + default: true, + description: 'Whether to include document metadata in the response', + }, + { + displayName: 'Include Metrics', + name: 'include_metrics', + type: 'boolean', + default: true, + description: 'Whether to include processing metrics in the response', + }, + { + displayName: 'Tags', + name: 'tags', + type: 'string', + default: '', + description: 'Comma-separated tags to associate with the document', + }, + { + displayName: 'Use Cached Results', + name: 'use_file_history', + type: 'boolean', + default: false, + description: 'Whether to use cached results if available. Useful while debugging your n8n workflow.', + }, + ], }, ], }; @@ -112,12 +171,12 @@ export class Unstract implements INodeType { try { const credentials = await this.getCredentials('unstractApi'); - const apiKey = credentials.apiKey as string; const orgId = credentials.orgId as string; const { helpers } = this; for (let i = 0; i < items.length; i++) { - const binaryPropertyName = this.getNodeParameter('file_contents', i) as string; + try { + const binaryPropertyName = this.getNodeParameter('file_contents', i) as string; if (!items[i].binary?.[binaryPropertyName]) { throw new NodeOperationError( @@ -132,10 +191,13 @@ export class Unstract implements INodeType { const timeout = this.getNodeParameter('timeout', i) as number; const deploymentName = this.getNodeParameter('deployment_name', i) as string; const host = this.getNodeParameter('host', i) as string; - const includeMetrics = this.getNodeParameter('include_metrics', i) as boolean; - const includeMetadata = this.getNodeParameter('include_metadata', i) as boolean; - const tags = this.getNodeParameter('tags', i) as string; - const useFileHistory = this.getNodeParameter('use_file_history', i) as boolean; + + // Get additional options with defaults + const additionalOptions = this.getNodeParameter('additionalOptions', i, {}) as any; + const includeMetrics = additionalOptions.include_metrics !== undefined ? additionalOptions.include_metrics : true; + const includeMetadata = additionalOptions.include_metadata !== undefined ? additionalOptions.include_metadata : true; + const tags = additionalOptions.tags || ''; + const useFileHistory = additionalOptions.use_file_history !== undefined ? additionalOptions.use_file_history : false; // Manual multipart/form-data construction (cloud-compatible, no external dependencies) // Workaround for n8n issue #18271 where httpRequestWithAuthentication doesn't properly @@ -184,7 +246,6 @@ export class Unstract implements INodeType { method: 'POST' as IHttpRequestMethods, url: `${host}/deployment/api/${orgId}/${deploymentName}/`, headers: { - 'Authorization': `Bearer ${apiKey}`, 'Content-Type': `multipart/form-data; boundary=${boundary}`, 'Content-Length': body.length.toString(), }, @@ -218,31 +279,28 @@ export class Unstract implements INodeType { const statusRequestOptions: any = { method: 'GET' as IHttpRequestMethods, url: `${host}${statusApi}`, - headers: { - 'Authorization': `Bearer ${apiKey}`, - }, timeout: 5 * 60 * 1000, + returnFullResponse: true, + ignoreHttpStatusErrors: true, }; + const statusResult = await helpers.httpRequestWithAuthentication.call(this, 'unstractApi', statusRequestOptions); + const statusCode = statusResult.statusCode || 200; + const body = statusResult.body || statusResult; + resultContent = typeof body === 'string' ? JSON.parse(body) : body; + executionStatus = resultContent.status; + + // HTTP 422 indicates execution still in progress - continue polling + if (statusCode === 422) { + continue; + } - try { - const statusResult = await helpers.httpRequest(statusRequestOptions); - resultContent = typeof statusResult === 'string' ? JSON.parse(statusResult) : statusResult; - executionStatus = resultContent.status; - } catch (error: any) { - // HTTP 422 indicates execution still in progress - this is expected - if (error.response?.status === 422 && error.response?.data) { - resultContent = typeof error.response.data === 'string' ? JSON.parse(error.response.data) : error.response.data; - executionStatus = resultContent.status; - } else { - // Actual error - log and rethrow - if (error.response?.status) { - } - throw new NodeOperationError( - this.getNode(), - `Failed to check execution status: ${error.message}`, - ); - } + // Handle other error status codes + if (statusCode >= 400 && statusCode !== 422) { + throw new NodeOperationError( + this.getNode(), + `Failed to check execution status: HTTP ${statusCode}`, + ); } const t2 = new Date(); @@ -264,6 +322,22 @@ export class Unstract implements INodeType { json: resultContent, pairedItem: { item: i }, }); + } catch (error: any) { + if (this.continueOnFail()) { + returnData.push({ + json: { + error: error.message || 'Unknown error occurred', + }, + pairedItem: { item: i }, + }); + continue; + } + + if (error.message) { + throw new NodeOperationError(this.getNode(), error.message); + } + throw error; + } } return [returnData]; diff --git a/nodes/UnstractHitlFetch/UnstractHitlFetch.node.ts b/nodes/UnstractHitlFetch/UnstractHitlFetch.node.ts index 74779c9..628584b 100644 --- a/nodes/UnstractHitlFetch/UnstractHitlFetch.node.ts +++ b/nodes/UnstractHitlFetch/UnstractHitlFetch.node.ts @@ -15,7 +15,8 @@ export class UnstractHitlFetch implements INodeType { icon: 'file:unstractHitlFetch.svg', group: ['transform'], version: 1, - description: 'Fetch final result from HITL queue using Unstract API', + usableAsTool: true, + description: 'Fetches approved documents from Unstract Human-in-the-Loop (HITL) queue after manual review', defaults: { name: 'Unstract HITL Fetch', }, @@ -28,10 +29,49 @@ export class UnstractHitlFetch implements INodeType { }, ], properties: [ + { + displayName: 'Resource', + name: 'resource', + type: 'options', + noDataExpression: true, + options: [ + { + name: 'Document', + value: 'document', + }, + ], + default: 'document', + }, + { + displayName: 'Operation', + name: 'operation', + type: 'options', + noDataExpression: true, + displayOptions: { + show: { + resource: ['document'], + }, + }, + options: [ + { + name: 'Fetch Approved', + value: 'fetchApproved', + description: 'Fetch approved documents from HITL queue', + action: 'Fetch approved document from HITL', + }, + ], + default: 'fetchApproved', + }, { displayName: 'Unstract Host', name: 'host', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['fetchApproved'], + }, + }, default: 'http://localhost:8000', required: true, }, @@ -39,6 +79,12 @@ export class UnstractHitlFetch implements INodeType { displayName: 'Workflow ID', name: 'workflow_id', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['fetchApproved'], + }, + }, default: '', required: true, }, @@ -46,7 +92,15 @@ export class UnstractHitlFetch implements INodeType { displayName: 'HITL Queue Name', name: 'hitl_queue_name', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['fetchApproved'], + }, + }, default: '', + required: true, + description: 'HITL queue name to filter results', }, ], }; @@ -72,6 +126,7 @@ export class UnstractHitlFetch implements INodeType { const options = { method: 'GET' as IHttpRequestMethods, url, + ignoreHttpStatusErrors: true, }; try { @@ -79,14 +134,10 @@ export class UnstractHitlFetch implements INodeType { const parsed = typeof response === 'string' ? JSON.parse(response) : response; if (parsed.error) { - if (parsed.error === 'No approved items available.') { - returnData.push({ - json: { message: 'No approved items available', hasData: false }, - pairedItem: { item: i } - }); - } else { - throw new NodeOperationError(this.getNode(), `API Error: ${parsed.error}`); - } + returnData.push({ + json: { error: parsed.error, hasData: false }, + pairedItem: { item: i } + }); } else if (parsed.data) { returnData.push({ json: { ...parsed.data, hasData: true }, @@ -96,6 +147,16 @@ export class UnstractHitlFetch implements INodeType { throw new NodeOperationError(this.getNode(), 'Unexpected response format.'); } } catch (error: any) { + if (this.continueOnFail()) { + returnData.push({ + json: { + error: error.message || 'Unknown error occurred', + }, + pairedItem: { item: i }, + }); + continue; + } + if (error.response && error.response.statusCode === 404) { throw new NodeOperationError(this.getNode(), 'Result not yet available (404)'); } diff --git a/nodes/UnstractHitlPush/UnstractHitlPush.node.ts b/nodes/UnstractHitlPush/UnstractHitlPush.node.ts index 3778baf..f0a2722 100644 --- a/nodes/UnstractHitlPush/UnstractHitlPush.node.ts +++ b/nodes/UnstractHitlPush/UnstractHitlPush.node.ts @@ -6,23 +6,9 @@ import { NodeOperationError, NodeConnectionType, IHttpRequestMethods, + sleep, } from 'n8n-workflow'; -const sleep = (ms: number): Promise => - new Promise((resolve) => { - // Use Promise-based delay that doesn't rely on restricted globals - const start = Date.now(); - const check = (): void => { - if (Date.now() - start >= ms) { - resolve(); - } else { - // Use Promise.resolve() for non-blocking delay - Promise.resolve().then(check); - } - }; - check(); - }); - export class UnstractHitlPush implements INodeType { description: INodeTypeDescription = { displayName: 'Unstract HITL Push', @@ -30,6 +16,7 @@ export class UnstractHitlPush implements INodeType { icon: 'file:unstractHitlPush.svg', group: ['transform'], version: 1, + usableAsTool: true, description: 'Push document for HITL processing using Unstract API', defaults: { name: 'Unstract HITL Push', @@ -43,10 +30,49 @@ export class UnstractHitlPush implements INodeType { }, ], properties: [ + { + displayName: 'Resource', + name: 'resource', + type: 'options', + noDataExpression: true, + options: [ + { + name: 'Document', + value: 'document', + }, + ], + default: 'document', + }, + { + displayName: 'Operation', + name: 'operation', + type: 'options', + noDataExpression: true, + displayOptions: { + show: { + resource: ['document'], + }, + }, + options: [ + { + name: 'Push to HITL', + value: 'push', + description: 'Push document for Human-in-the-Loop processing', + action: 'Push document to HITL queue', + }, + ], + default: 'push', + }, { displayName: 'File Contents', name: 'file_contents', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['push'], + }, + }, default: 'data', description: 'Name of the binary property containing file data', required: true, @@ -55,6 +81,12 @@ export class UnstractHitlPush implements INodeType { displayName: 'Unstract Host', name: 'host', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['push'], + }, + }, default: 'https://us-central.unstract.com', required: true, }, @@ -62,6 +94,12 @@ export class UnstractHitlPush implements INodeType { displayName: 'API Deployment Name', name: 'deployment_name', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['push'], + }, + }, default: '', required: true, }, @@ -69,6 +107,12 @@ export class UnstractHitlPush implements INodeType { displayName: 'HITL Queue Name', name: 'hitl_queue_name', type: 'string', + displayOptions: { + show: { + resource: ['document'], + operation: ['push'], + }, + }, default: '', required: true, }, @@ -76,34 +120,58 @@ export class UnstractHitlPush implements INodeType { displayName: 'Timeout', name: 'timeout', type: 'number', + displayOptions: { + show: { + resource: ['document'], + operation: ['push'], + }, + }, default: 600, description: 'Max seconds to wait for processing', required: true, }, { - displayName: 'Include Metrics', - name: 'include_metrics', - type: 'boolean', - default: true, - }, - { - displayName: 'Include Metadata', - name: 'include_metadata', - type: 'boolean', - default: true, - }, - { - displayName: 'Tags', - name: 'tags', - type: 'string', - default: '', - description: 'Comma-separated tags for the document', - }, - { - displayName: 'Use Cached Results', - name: 'use_file_history', - type: 'boolean', - default: false, + displayName: 'Additional Options', + name: 'additionalOptions', + type: 'collection', + placeholder: 'Add Option', + displayOptions: { + show: { + resource: ['document'], + operation: ['push'], + }, + }, + default: {}, + options: [ + { + displayName: 'Include Metadata', + name: 'include_metadata', + type: 'boolean', + default: true, + description: 'Whether to include document metadata in the response', + }, + { + displayName: 'Include Metrics', + name: 'include_metrics', + type: 'boolean', + default: true, + description: 'Whether to include processing metrics in the response', + }, + { + displayName: 'Tags', + name: 'tags', + type: 'string', + default: '', + description: 'Comma-separated tags for the document', + }, + { + displayName: 'Use Cached Results', + name: 'use_file_history', + type: 'boolean', + default: false, + description: 'Whether to use cached results if available', + }, + ], }, ], }; @@ -114,12 +182,12 @@ export class UnstractHitlPush implements INodeType { try { const credentials = await this.getCredentials('unstractApi'); - const apiKey = credentials.apiKey as string; const orgId = credentials.orgId as string; const { helpers } = this; for (let i = 0; i < items.length; i++) { - const binaryPropertyName = this.getNodeParameter('file_contents', i) as string; + try { + const binaryPropertyName = this.getNodeParameter('file_contents', i) as string; if (!items[i].binary?.[binaryPropertyName]) { throw new NodeOperationError(this.getNode(), `No binary data property "${binaryPropertyName}" exists on input`); @@ -131,12 +199,15 @@ export class UnstractHitlPush implements INodeType { const timeout = this.getNodeParameter('timeout', i) as number; const deploymentName = this.getNodeParameter('deployment_name', i) as string; const host = this.getNodeParameter('host', i) as string; - const includeMetrics = this.getNodeParameter('include_metrics', i) as boolean; - const includeMetadata = this.getNodeParameter('include_metadata', i) as boolean; - const tags = this.getNodeParameter('tags', i) as string; - const useFileHistory = this.getNodeParameter('use_file_history', i) as boolean; const hitlQueueName = this.getNodeParameter('hitl_queue_name', i) as string; + // Get additional options with defaults + const additionalOptions = this.getNodeParameter('additionalOptions', i, {}) as any; + const includeMetrics = additionalOptions.include_metrics !== undefined ? additionalOptions.include_metrics : true; + const includeMetadata = additionalOptions.include_metadata !== undefined ? additionalOptions.include_metadata : true; + const tags = additionalOptions.tags || ''; + const useFileHistory = additionalOptions.use_file_history !== undefined ? additionalOptions.use_file_history : false; + // Manual multipart/form-data construction (cloud-compatible, no external dependencies) // Workaround for n8n issue #18271 where httpRequestWithAuthentication doesn't properly // handle formData objects. See: https://github.com/n8n-io/n8n/issues/18271 @@ -185,7 +256,6 @@ export class UnstractHitlPush implements INodeType { method: 'POST' as IHttpRequestMethods, url: `${host}/deployment/api/${orgId}/${deploymentName}/`, headers: { - 'Authorization': `Bearer ${apiKey}`, 'Content-Type': `multipart/form-data; boundary=${boundary}`, 'Content-Length': body.length.toString(), }, @@ -219,30 +289,28 @@ export class UnstractHitlPush implements INodeType { const statusRequestOptions: any = { method: 'GET' as IHttpRequestMethods, url: `${host}${statusApi}`, - headers: { - 'Authorization': `Bearer ${apiKey}`, - }, timeout: 5 * 60 * 1000, + returnFullResponse: true, + ignoreHttpStatusErrors: true, }; - try { - const statusResult = await helpers.httpRequest(statusRequestOptions); - resultContent = typeof statusResult === 'string' ? JSON.parse(statusResult) : statusResult; - executionStatus = resultContent.status; - } catch (error: any) { - // HTTP 422 indicates execution still in progress - this is expected - if (error.response?.status === 422 && error.response?.data) { - resultContent = typeof error.response.data === 'string' ? JSON.parse(error.response.data) : error.response.data; - executionStatus = resultContent.status; - } else { - // Actual error - log and rethrow - if (error.response?.status) { - } - throw new NodeOperationError( - this.getNode(), - `Failed to check execution status: ${error.message}`, - ); - } + const statusResult = await helpers.httpRequestWithAuthentication.call(this, 'unstractApi', statusRequestOptions); + const statusCode = statusResult.statusCode || 200; + const body = statusResult.body || statusResult; + resultContent = typeof body === 'string' ? JSON.parse(body) : body; + executionStatus = resultContent.status; + + // HTTP 422 indicates execution still in progress - continue polling + if (statusCode === 422) { + continue; + } + + // Handle other error status codes + if (statusCode >= 400 && statusCode !== 422) { + throw new NodeOperationError( + this.getNode(), + `Failed to check execution status: HTTP ${statusCode}`, + ); } const t2 = new Date(); @@ -264,6 +332,22 @@ export class UnstractHitlPush implements INodeType { json: resultContent, pairedItem: { item: i } }); + } catch (error: any) { + if (this.continueOnFail()) { + returnData.push({ + json: { + error: error.message || 'Unknown error occurred', + }, + pairedItem: { item: i }, + }); + continue; + } + + if (error.message) { + throw new NodeOperationError(this.getNode(), error.message); + } + throw error; + } } return [returnData];