Skip to content
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

fix: add more error info on missing functions folders #1134

Merged
merged 3 commits into from Jul 18, 2022

Conversation

danez
Copy link
Contributor

@danez danez commented Jul 5, 2022

🎉 Thanks for submitting a pull request! 🎉

Summary

For https://github.com/netlify/pod-compute/issues/108

Throws underlying errors if the error is not a FileNotFound Error.

@danez danez added the bug Something isn't working label Jul 5, 2022
@github-actions github-actions bot added the type: bug code to address defects in shipped code label Jul 5, 2022
export const listFunctionsDirectory = async function (srcFolder: string) {
try {
const filenames = await fs.readdir(srcFolder)
const listFunctionsDirectory = async function (srcFolder: string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the export as it is nowhere used outside this file, and also is not a Public API.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2022

⏱ Benchmark results

Comparing with cd082fc

largeDepsEsbuild: 5.7s

⬇️ 4.85% decrease vs. cd082fc

^                   6.2s                                  
│   5.9s    5.9s    ┌──┐    5.8s     6s      6s           
│ ──┌──┐────┌──┐────┼──┼────┌──┐────┌──┐────┌──┐────5.7s──
│   |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

largeDepsNft: 28s

⬇️ 6.32% decrease vs. cd082fc

^  29.2s   29.1s                           29.8s          
│   ┌──┐    ┌──┐                            ┌──┐    28s   
│ ──┼──┼────┼──┼───────────26.8s────────────┼──┼────┌──┐──
│   |  |    |  |            ┌──┐            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |            |  |            |  |    |▒▒|  
│   |  |    |  |    n/a     |  |    n/a     |  |    |▒▒|  
└───┴──┴────┴──┴────────────┴──┴────────────┴──┴────┴──┴──>
    T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

largeDepsZisi: 42.9s

⬇️ 4.47% decrease vs. cd082fc

^                  45.7s                   44.8s          
│  43.7s            ┌──┐            44s     ┌──┐   42.9s  
│ ──┌──┐────────────┼──┼───41.8s────┌──┐────┼──┼────┌──┐──
│   |  |            |  |    ┌──┐    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |            |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    n/a     |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────────────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

return filenames.map((name) => join(srcFolder, name))
} catch {
throw new Error(`Functions folder does not exist: ${srcFolder}`)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we do not export it we do not need to catch anymore, because the error will be matched in the function above.

srcFolders.map(async (srcFolder) => {
try {
const filenames = await listFunctionsDirectory(srcFolder)
const filenamesByDirectory = await Promise.allSettled(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allSettled is available as of node 12.9.0

@danez danez removed the bug Something isn't working label Jul 5, 2022
@danez danez requested a review from a team July 7, 2022 08:18
src/utils/fs.ts Outdated
const validDirectories = filenamesByDirectory
.map((result) => {
if (result.status === 'rejected') {
if (result.reason instanceof Error && (result.reason as NodeJS.ErrnoException).code !== 'ENOENT') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason Error does not contain code, so this is the workaround.

src/utils/fs.ts Outdated Show resolved Hide resolved
@danez danez requested a review from eduardoboucas July 18, 2022 12:05
@kodiakhq kodiakhq bot merged commit b0048a4 into main Jul 18, 2022
@kodiakhq kodiakhq bot deleted the fix/danez/missing-folder branch July 18, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants