chore(objects upload): improves response when error due to large object size (#1685)
* test(objects upload): adds a test for large object * print object too large messages in response * allows object maximum size to be configured
This commit is contained in:
@@ -60,8 +60,10 @@ FROM node:18.16.1-bullseye-slim as production-stage
|
||||
ARG NODE_ENV
|
||||
ARG SPECKLE_SERVER_VERSION
|
||||
ARG FILE_SIZE_LIMIT_MB=100
|
||||
ARG MAX_OBJECT_SIZE_MB=10
|
||||
|
||||
ENV FILE_SIZE_LIMIT_MB=${FILE_SIZE_LIMIT_MB} \
|
||||
MAX_OBJECT_SIZE_MB=${MAX_OBJECT_SIZE_MB} \
|
||||
NODE_ENV=${NODE_ENV} \
|
||||
SPECKLE_SERVER_VERSION=${SPECKLE_SERVER_VERSION}
|
||||
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
import { BaseError } from '@/modules/shared/errors'
|
||||
import { Options } from 'verror'
|
||||
|
||||
export class ObjectHandlingError extends BaseError {
|
||||
static defaultMessage = 'Failed to handle object.'
|
||||
static code = 'OBJECT_HANDLING_ERROR'
|
||||
|
||||
constructor(message?: string | undefined, options?: Options | Error | undefined) {
|
||||
super(message, options)
|
||||
}
|
||||
}
|
||||
@@ -5,7 +5,8 @@ const Busboy = require('busboy')
|
||||
|
||||
const { validatePermissionsWriteStream } = require('./authUtils')
|
||||
|
||||
const { createObjectsBatched } = require('../services/objects')
|
||||
const { createObjectsBatched } = require('@/modules/core/services/objects')
|
||||
const { ObjectHandlingError } = require('@/modules/core/errors/object')
|
||||
|
||||
const MAX_FILE_SIZE = 50 * 1024 * 1024
|
||||
|
||||
@@ -113,12 +114,21 @@ module.exports = (app) => {
|
||||
|
||||
const promise = createObjectsBatched(req.params.streamId, objs).catch((e) => {
|
||||
req.log.error(e, `Upload error.`)
|
||||
if (!requestDropped)
|
||||
res
|
||||
.status(400)
|
||||
.send(
|
||||
'Error inserting object in the database. Check server logs for details'
|
||||
)
|
||||
if (!requestDropped) {
|
||||
switch (e.constructor) {
|
||||
case ObjectHandlingError:
|
||||
res
|
||||
.status(400)
|
||||
.send(`Error inserting object in the database: ${e.message}`)
|
||||
break
|
||||
default:
|
||||
res
|
||||
.status(400)
|
||||
.send(
|
||||
'Error inserting object in the database. Check server logs for details'
|
||||
)
|
||||
}
|
||||
}
|
||||
requestDropped = true
|
||||
})
|
||||
promises.push(promise)
|
||||
@@ -194,11 +204,19 @@ module.exports = (app) => {
|
||||
const promise = createObjectsBatched(req.params.streamId, objs).catch((e) => {
|
||||
req.log.error(e, `Upload error.`)
|
||||
if (!requestDropped)
|
||||
res
|
||||
.status(400)
|
||||
.send(
|
||||
'Error inserting object in the database. Check server logs for details'
|
||||
)
|
||||
switch (e.constructor) {
|
||||
case ObjectHandlingError:
|
||||
res
|
||||
.status(400)
|
||||
.send(`Error inserting object in the database. ${e.message}`)
|
||||
break
|
||||
default:
|
||||
res
|
||||
.status(400)
|
||||
.send(
|
||||
'Error inserting object in the database. Check server logs for details'
|
||||
)
|
||||
}
|
||||
requestDropped = true
|
||||
})
|
||||
promises.push(promise)
|
||||
|
||||
@@ -5,6 +5,8 @@ const { set, get, chunk } = require('lodash')
|
||||
|
||||
const knex = require(`@/db/knex`)
|
||||
const { servicesLogger } = require('@/logging/logging')
|
||||
const { getMaximumObjectSizeMB } = require('@/modules/shared/helpers/envHelper')
|
||||
const { ObjectHandlingError } = require('@/modules/core/errors/object')
|
||||
|
||||
const Objects = () => knex('objects')
|
||||
const Closures = () => knex('object_children_closure')
|
||||
@@ -586,7 +588,7 @@ module.exports = {
|
||||
|
||||
// NOTE: Derive Object
|
||||
async updateObject() {
|
||||
throw new Error('not implemeneted')
|
||||
throw new Error('Updating object is not implemented')
|
||||
}
|
||||
}
|
||||
|
||||
@@ -595,7 +597,7 @@ module.exports = {
|
||||
// we cannot provide a full response back including all object hashes.
|
||||
function prepInsertionObject(streamId, obj) {
|
||||
// let memNow = process.memoryUsage().heapUsed / 1024 / 1024
|
||||
const MAX_OBJECT_SIZE = 10 * 1024 * 1024
|
||||
const MAX_OBJECT_SIZE = getMaximumObjectSizeMB() * 1024 * 1024
|
||||
|
||||
if (obj.hash) obj.id = obj.hash
|
||||
else
|
||||
@@ -604,7 +606,9 @@ function prepInsertionObject(streamId, obj) {
|
||||
|
||||
const stringifiedObj = JSON.stringify(obj)
|
||||
if (stringifiedObj.length > MAX_OBJECT_SIZE) {
|
||||
throw new Error(`Object too large (${stringifiedObj.length} > ${MAX_OBJECT_SIZE})`)
|
||||
throw new ObjectHandlingError(
|
||||
`Object too large. (${stringifiedObj.length} > ${MAX_OBJECT_SIZE})`
|
||||
)
|
||||
}
|
||||
// let memAfter = process.memoryUsage().heapUsed / 1024 / 1024
|
||||
|
||||
|
||||
@@ -237,6 +237,22 @@ describe('Upload/Download Routes @api-rest', () => {
|
||||
expect(res).to.have.status(400)
|
||||
})
|
||||
|
||||
it('Should not allow upload with invalid body (object too large)', async () => {
|
||||
//creating a single valid object larger than 10MB
|
||||
const objectToPost = {
|
||||
name: 'x'.repeat(10 * 1024 * 1024 + 1)
|
||||
}
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/objects/${testStream.id}`)
|
||||
.set('Authorization', userA.token)
|
||||
.set('Content-type', 'multipart/form-data')
|
||||
.attach('batch1', Buffer.from(JSON.stringify([objectToPost]), 'utf8'))
|
||||
|
||||
expect(res).to.have.status(400)
|
||||
expect(res.text).contains('Object too large')
|
||||
})
|
||||
|
||||
let parentId
|
||||
const numObjs = 5000
|
||||
const objBatches = [
|
||||
|
||||
@@ -37,6 +37,10 @@ export function getFileSizeLimitMB() {
|
||||
return getIntFromEnv('FILE_SIZE_LIMIT_MB', '100')
|
||||
}
|
||||
|
||||
export function getMaximumObjectSizeMB() {
|
||||
return getIntFromEnv('MAX_OBJECT_SIZE_MB', '10')
|
||||
}
|
||||
|
||||
export function getIntFromEnv(envVarKey: string, aDefault = '0'): number {
|
||||
return parseInt(process.env[envVarKey] || aDefault)
|
||||
}
|
||||
|
||||
@@ -107,7 +107,7 @@ spec:
|
||||
{{- else }}
|
||||
value: http://{{ .Values.domain }}
|
||||
{{- end }}
|
||||
|
||||
|
||||
- name: ONBOARDING_STREAM_ID
|
||||
value: {{ .Values.server.onboardingStreamId }}
|
||||
|
||||
@@ -120,6 +120,9 @@ spec:
|
||||
- name: FILE_SIZE_LIMIT_MB
|
||||
value: {{ .Values.file_size_limit_mb | quote }}
|
||||
|
||||
- name: MAX_OBJECT_SIZE_MB
|
||||
value: {{ .Values.server.max_object_size_mb | quote }}
|
||||
|
||||
- name: SPECKLE_AUTOMATE_URL
|
||||
value: {{ .Values.server.speckleAutomateUrl }}
|
||||
|
||||
@@ -276,7 +279,7 @@ spec:
|
||||
- name: EMAIL_FROM
|
||||
value: "{{ .Values.server.email.from }}"
|
||||
{{- end }}
|
||||
|
||||
|
||||
# *** Newsletter ***
|
||||
{{- if (default false (.Values.server.mailchimp).enabled) }}
|
||||
- name: MAILCHIMP_ENABLED
|
||||
|
||||
@@ -491,6 +491,11 @@
|
||||
"description": "The id of the stream to be used for onboarding new users",
|
||||
"default": ""
|
||||
},
|
||||
"max_object_size_mb": {
|
||||
"type": "number",
|
||||
"description": "The maximum size of an individual object which can be uploaded to the server",
|
||||
"default": 10
|
||||
},
|
||||
"speckleAutomateUrl": {
|
||||
"type": "string",
|
||||
"description": "The url of the Speckle Automate instance",
|
||||
|
||||
@@ -387,6 +387,8 @@ server:
|
||||
adminOverrideEnabled: false
|
||||
## @param server.onboardingStreamId The id of the stream to be used for onboarding new users
|
||||
onboardingStreamId: ''
|
||||
## @param server.max_object_size_mb The maximum size of an individual object which can be uploaded to the server
|
||||
max_object_size_mb: 10
|
||||
## @param server.speckleAutomateUrl The url of the Speckle Automate instance
|
||||
speckleAutomateUrl: 'https://automate.speckle.systems'
|
||||
sessionSecret:
|
||||
|
||||
Reference in New Issue
Block a user