fix(gatsby-plugin-sharp): don't write to same temporary file multiple times at a same time #12927

pieh · 2019-03-28T22:50:05Z

Memoization key for tracedSVG was not entirely correct and we were trying to write to temporary file multiple times. This was causing those temporary files to become corrupted in dependencies of potrace were crashing while trying to read those corrupted input files (there was a lot of concurrency, so that's why those crashes were very random - sometimes timing was bad and we would end up . This should fix memoization key and avoid writing to same file more than once - there is also extra memoization layer just for that, so if general memoization key for traceSVG function fails, there is another memoization that checks just temporary file location.

This was happening most likely when there were copies of same image in multiple places (as memoization key was using absolutePath before).

I've moved some code around (mostly because I needed to export some trace SVG related functions for added tests). I'll try to mark places in removed code that did change.

related issue #8301

pieh · 2019-03-28T22:51:22Z

packages/gatsby-plugin-sharp/src/index.js

-
-const memoizedTraceSVG = _.memoize(
-  notMemoizedtraceSVG,
-  ({ file, args }) => `${file.absolutePath}${JSON.stringify(args)}`


This was changed to

({ file, args, fileArgs }) => `${file.internal.contentDigest}${JSON.stringify(args)}${JSON.stringify(fileArgs)}`

pieh · 2019-03-28T22:52:06Z

packages/gatsby-plugin-sharp/src/index.js

-    pipeline.toFile(tmpFilePath, (err, info) => {
-      resolve()
-    })
-  )


generation of temporary input file was refactored into separate function notMemoizedPrepareTraceSVGInputFile and added memoization on top that use tmpFilePath as key

tylermenezes · 2019-03-28T23:38:45Z

(This doesn't fully fix #8301 -- there are a bunch of other reasons you could get that error.)

pieh · 2019-03-29T00:27:08Z

@tylermenezes can you expand on that? I know you can get this in bunch of different ways, but what are other things we can fix (if you have that context).

I scanned through comments in that issue and there was lot of guesses, but not a lot of actual details.

I've also added throw if for some reason sharp throws when generating this temporary image file, so this causing problem would be potentially eliminated.

I tried using .jpeg is input file and it worked for me locally.

tylermenezes · 2019-03-29T00:40:01Z

Basically "media files potrace supports" is a subset of "media files". I'm still not totally sure why my original test case wasn't supported (maybe it was because it also had a colon in the name), but I troubleshooted this as a fix and it didn't solve it.

This is a useful fix for many people, I just think it's premature to call the original issue really fixed, when the error message is so generic that literally any problem will cause it.

pieh · 2019-03-29T01:14:27Z

Basically "media files potrace supports" is a subset of "media files". I'm still not totally sure why my original test case wasn't supported (maybe it was because it also had a colon in the name), but I troubleshooted this as a fix and it didn't solve it.

Can you share your reproduction test case? I'm not sure if this is about .jpeg extension, but I did try using those even before the fix and it was working, so it might more file specific?

This is a useful fix for many people, I just think it's premature to call the original issue really fixed, when the error message is so generic that literally any problem will cause it.

Sure, no argument here, I'll change my PR description to not autoclose linked issue, and will post message there so people update and report back.

DSchau · 2019-03-29T13:30:02Z

@tylermenezes just to be clear, you tried this fix out with your example--and still running into errors?

I know we asked in the original thread, but what would be most helpful here is if we could access that reproduction. Feel free to DM me on twitter if any secret details could be shared. We haven't really had a reproducible example to craft this fix--so anything towards that end will ensure we're able to fix each occurrence of this issue.

Thank you!

DSchau · 2019-04-09T14:52:18Z

@tylermenezes a gentle reminder that if you could provide any more detail as to why this fix doesn't work for you (and showing with a reproduction or even more information) that would be very much appreciated.

We think this does fix the issue for most (all?), so if you have evidence to the contrary we'd love to ensure we do fix it for all.

tylermenezes · 2019-04-09T15:49:08Z

You're welcome to close it if you're convinced it will fix the issue. I didn't save any images that caused errors so I'm not able to provide the same reproduction I had last time.

DSchau

These tests are going to give me nightmares 😱 Just kidding!

This looks great--let's get it merged in, and we'll monitor errors and issues to see if we see this thing continue to pop up.

… times at a same time (gatsbyjs#12927) Memoization key for tracedSVG was not entirely correct and we were trying to write to temporary file multiple times. This was causing those temporary files to become corrupted in dependencies of `potrace` were crashing while trying to read those corrupted input files (there was a lot of concurrency, so that's why those crashes were very random - sometimes timing was bad and we would end up . This _should_ fix memoization key and avoid writing to same file more than once - there is also extra memoization layer just for that, so if general memoization key for `traceSVG` function fails, there is another memoization that checks just temporary file location. This was happening most likely when there were copies of same image in multiple places (as memoization key was using `absolutePath` before). I've moved some code around (mostly because I needed to export some trace SVG related functions for added tests). I'll try to mark places in removed code that did change. related issue gatsbyjs#8301

marcosilvestroni · 2019-05-20T15:18:22Z

Hi, Maybe It could help to find an other usecase, I've got the same problem here while I'm running in develop and then try to build the project, and vice versa. I have to clean the directrories to make it works or remove the tracedSVG from my gasby-node.js query while generating pages

isAlmogK · 2020-09-21T00:00:16Z

Has this been fixed I'm getting the same issue?

fraserisland · 2021-01-26T03:44:50Z

I'm getting this issue when querying for tracedSVG's, i'm getting different errors depending on the file type. It would be great to find a solution, especially with how beneficial tracedSVG's can be in lighthouse v6 scores against blur-up.

PNG's work with no errors.
WebP images will crash the builds with: TypeError: Cannot read property 'bitmap' of undefined at Potrace._processLoadedImage.
JPEG's throw this error: "VipsJpeg: Premature end of JPEG file\nVipsJpeg: out of order read at line 224\n", however don't crash the build.

"gatsby-source-contentful": "4.4.1",
"gatsby-transformer-sharp": "2.10.1",
"gatsby-plugin-sharp": "2.12.2",
"gatsby": "2.30.3",
"gatsby-image": "2.9.0",

fix(gatsby-plugin-sharp): should fix cannot read bitmap of undefined

9429899

pieh commented Mar 28, 2019

View reviewed changes

This was referenced Mar 28, 2019

gatsby-plugin-sharp tracedSVG crashes build #8301

Closed

[bug]: potrace fails randomly with "cannot read property bitmap of undefined" #12552

Closed

pieh changed the title ~~fix(gatsby-plugin-sharp): should fix cannot read bitmap of undefined~~ fix(gatsby-plugin-sharp): don't write to same temporary time multiple times at a same time Mar 29, 2019

Merge remote-tracking branch 'upstream/master' into potrace-stuff

da05b81

sidharthachatterjee mentioned this pull request Apr 2, 2019

Improve error messaging for sharp #13026

Closed

3 tasks

DSchau added bot: merge on green Gatsbot will merge these PRs automatically when all tests passes status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response labels Apr 9, 2019

DSchau approved these changes Apr 9, 2019

View reviewed changes

DSchau merged commit 5e254e2 into gatsbyjs:master Apr 9, 2019

KyleAMathews mentioned this pull request Apr 20, 2019

chore(gatsby-transformer-sqip): improve sqip performance and debug logging #13349

Merged

pieh deleted the potrace-stuff branch April 23, 2019 12:10

CanRau changed the title ~~fix(gatsby-plugin-sharp): don't write to same temporary time multiple times at a same time~~ fix(gatsby-plugin-sharp): don't write to same temporary file multiple times at a same time Apr 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(gatsby-plugin-sharp): don't write to same temporary file multiple times at a same time #12927

fix(gatsby-plugin-sharp): don't write to same temporary file multiple times at a same time #12927

pieh commented Mar 28, 2019 •

edited

Loading

pieh Mar 28, 2019

pieh Mar 28, 2019

tylermenezes commented Mar 28, 2019

pieh commented Mar 29, 2019 •

edited

Loading

tylermenezes commented Mar 29, 2019

pieh commented Mar 29, 2019

DSchau commented Mar 29, 2019

DSchau commented Apr 9, 2019

tylermenezes commented Apr 9, 2019

DSchau left a comment

marcosilvestroni commented May 20, 2019

isAlmogK commented Sep 21, 2020

fraserisland commented Jan 26, 2021 •

edited

Loading

fix(gatsby-plugin-sharp): don't write to same temporary file multiple times at a same time #12927

fix(gatsby-plugin-sharp): don't write to same temporary file multiple times at a same time #12927

Conversation

pieh commented Mar 28, 2019 • edited Loading

pieh Mar 28, 2019

Choose a reason for hiding this comment

pieh Mar 28, 2019

Choose a reason for hiding this comment

tylermenezes commented Mar 28, 2019

pieh commented Mar 29, 2019 • edited Loading

tylermenezes commented Mar 29, 2019

pieh commented Mar 29, 2019

DSchau commented Mar 29, 2019

DSchau commented Apr 9, 2019

tylermenezes commented Apr 9, 2019

DSchau left a comment

Choose a reason for hiding this comment

marcosilvestroni commented May 20, 2019

isAlmogK commented Sep 21, 2020

fraserisland commented Jan 26, 2021 • edited Loading

pieh commented Mar 28, 2019 •

edited

Loading

pieh commented Mar 29, 2019 •

edited

Loading

fraserisland commented Jan 26, 2021 •

edited

Loading