Skip to content

Commit

Permalink
summary: remove limit validation in client
Browse files Browse the repository at this point in the history
  • Loading branch information
robherley committed Apr 20, 2022
1 parent ed87cc6 commit eef3e92
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 48 deletions.
13 changes: 1 addition & 12 deletions packages/core/__tests__/markdown-summary.test.ts
@@ -1,11 +1,7 @@
import * as fs from 'fs'
import * as os from 'os'
import path from 'path'
import {
markdownSummary,
SUMMARY_ENV_VAR,
SUMMARY_LIMIT_BYTES
} from '../src/markdown-summary'
import {markdownSummary, SUMMARY_ENV_VAR} from '../src/markdown-summary'

const testFilePath = path.join(__dirname, 'test', 'test-summary.md')

Expand Down Expand Up @@ -96,13 +92,6 @@ describe('@actions/core/src/markdown-summary', () => {
await expect(write).rejects.toThrow()
})

it('throws if write will exceed file limit', async () => {
const aaa = 'a'.repeat(SUMMARY_LIMIT_BYTES + 1)
const write = markdownSummary.addRaw(aaa).write()

await expect(write).rejects.toThrow()
})

it('appends text to summary file', async () => {
await fs.promises.writeFile(testFilePath, '# ', {encoding: 'utf8'})
await markdownSummary.addRaw(fixtures.text).write()
Expand Down
38 changes: 2 additions & 36 deletions packages/core/src/markdown-summary.ts
@@ -1,9 +1,7 @@
import {EOL} from 'os'
import {constants, promises} from 'fs'
const {access, appendFile, stat, writeFile} = promises
const {access, appendFile, writeFile} = promises

// The runner & server will also block any upload greater than this size
export const SUMMARY_LIMIT_BYTES = 1024 * 1024 // 1MiB
export const SUMMARY_ENV_VAR = 'GITHUB_STEP_SUMMARY'
export const SUMMARY_DOCS_URL =
'https://docs.github.com/actions/using-workflows/workflow-commands-for-github-actions#adding-a-markdown-summary'
Expand Down Expand Up @@ -91,28 +89,6 @@ class MarkdownSummary {
return this._filePath
}

/**
* Checks if the write of the current buffer will exceed the job summary upload limit
*
* @param {boolean} overwrite if the operation is overwrite (otherwise it's append)
*
* @returns {Promise<boolean>} whether or not the file will exceed the limit
*/
private async willExceedLimit(overwrite: boolean): Promise<boolean> {
let expectedSize = 0
if (!overwrite) {
// if appending, we need to check the current size of the summary file
const filePath = await this.filePath()
const {size} = await stat(filePath)
expectedSize += size
}

const bufferLen = Buffer.byteLength(this._buffer, 'utf8')
expectedSize += bufferLen

return expectedSize > SUMMARY_LIMIT_BYTES
}

/**
* Wraps content in an HTML tag, adding any HTML attributes
*
Expand Down Expand Up @@ -140,24 +116,14 @@ class MarkdownSummary {

/**
* Writes text in the buffer to the summary buffer file and empties buffer. Will append by default.
* Checks if resulting file size > SUMMARY_LIMIT_BYTES, will throw and empty buffer
*
* @param {SummaryWriteOptions | undefined} options (optional) options for write operation
* @param {SummaryWriteOptions} [options] (optional) options for write operation
*
* @returns {Promise<MarkdownSummary>} markdown summary instance
*/
async write(options?: SummaryWriteOptions): Promise<MarkdownSummary> {
const overwrite = !!options?.overwrite
const filePath = await this.filePath()

if (await this.willExceedLimit(overwrite)) {
this.emptyBuffer()
const limitK = SUMMARY_LIMIT_BYTES / 1024
throw new Error(
`Aborting write to summary file. File size would exceed limit of ${limitK}KiB. For more information see: ${SUMMARY_DOCS_URL}`
)
}

const writeFunc = overwrite ? writeFile : appendFile
await writeFunc(filePath, this._buffer, {encoding: 'utf8'})
return this.emptyBuffer()
Expand Down

0 comments on commit eef3e92

Please sign in to comment.