New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Localizable errors, early return, specify maxSize #5

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@mmckegg
Member

mmckegg commented Dec 3, 2018

This is a bit of a grab bag of my latest changes to ssb-blob-files for patchwork. The code was a bit tangled up, so I'm pushing as one big ball LOL (it's all good stuff though!)

localisable error messages

The PR adds a custom error type MaxSizeError with fileName, fileSize and maxFileSize properties.

These can be used when localising the error message, for example:

if (err instanceof blobFiles.MaxSizeError) {
  warningMessage.set([
    i18n('{{name}} ({{size}}) is larger than the allowed limit of {{max_size}}', {
      'name': err.fileName,
      'size': humanSize(err.fileSize),
      'max_size': humanSize(err.maxFileSize)
    })
  ])
}

early return

Currently if you attach a very large file (100 MB or so), this may lock up and even crash the client due to the use of reader.readAsDataURL(file) which loads the entire file into an in memory string. If we want to support large files later, we'll need to update the code to use streams. Since we only allow 5 MB files, we should just error before trying to process the file instead.

The PR adds an early return if resize is not specified and the fileSize exceeds maxSize.

specify maxSize

And finally, this adds an option to specify maxSize since as @regular points out, this is configurable in sbot!


closes #3
maybe also #4 ??
see also ssbc/patchwork#893

mmckegg added some commits Dec 3, 2018

add localisable error messages
err.fileName, err.fileSize, err.code === 'EXCEEDS_MAX_SIZE'
err instanceof require('ssb-blob-files').MaxSizeError
add early return path for files exceeding maxSize
instead of crashing the client!!

@mmckegg mmckegg requested review from mixmix and christianbundy Dec 3, 2018

@mmckegg mmckegg changed the title from Localizable errors and early return to Localizable errors, early return, specify maxSize Dec 3, 2018

@christianbundy

This LGTM, it looks like you did all the right things:

  • Replace new Buffer with Buffer.from
  • Check for maximum size on publish
  • Created a very descriptive file for the new error
  • Expressed the error in a human-readable file size (alternatively: prettysize)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment